github/overleaf
1
0
Fork 0
mirror of https://github.com/overleaf/overleaf.git synced 2024-11-21 20:47:08 -05:00
Code Issues Projects Releases Packages Wiki Activity

Merge pull request #99 from overleaf/jpa-drop-delete-doc-endpoint

Browse source 
[misc] drop the deleteDoc endpoint -- use patchDoc instead
This commit is contained in:
Jakob Ackermann 2021-04-13 14:16:26 +02:00 committed by GitHub
commit 3ac7cdf433
9 changed files with 3 additions and 335 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

4
services/docstore/app.js
View file

@ -72,7 +72,9 @@ app.patch(
}),
HttpController.patchDoc
)
app.delete('/project/:project_id/doc/:doc_id', HttpController.deleteDoc)
app.delete('/project/:project_id/doc/:doc_id', (req, res) => {
res.status(500).send('DELETE-ing a doc is DEPRECATED. PATCH the doc instead.')
})
app.post('/project/:project_id/archive', HttpController.archiveAllDocs)
app.post('/project/:project_id/doc/:doc_id/archive', HttpController.archiveDoc)

52
services/docstore/app/js/DocManager.js
View file

@ -74,23 +74,6 @@ module.exports = DocManager = {
})
},
checkDocExists(project_id, doc_id, callback) {
if (callback == null) {
callback = function (err, exists) {}
}
return DocManager._getDoc(
project_id,
doc_id,
{ _id: 1, inS3: true },
function (err, doc) {
if (err != null) {
return callback(err)
}
return callback(err, doc != null)
}
)
},
isDocDeleted(projectId, docId, callback) {
MongoManager.findDoc(projectId, docId, { deleted: true }, function (
err,
@ -291,41 +274,6 @@ module.exports = DocManager = {
)
},
deleteDoc(project_id, doc_id, callback) {
if (callback == null) {
callback = function (error) {}
}
return DocManager.checkDocExists(project_id, doc_id, function (
error,
exists
) {
if (error != null) {
return callback(error)
}
if (!exists) {
return callback(
new Errors.NotFoundError(
`No such project/doc to delete: ${project_id}/${doc_id}`
)
)
}
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)
})
},
patchDoc(project_id, doc_id, meta, callback) {
const projection = { _id: 1, deleted: true }
MongoManager.findDoc(project_id, doc_id, projection, (error, doc) => {

15
services/docstore/app/js/HttpController.js
View file

@ -191,21 +191,6 @@ module.exports = HttpController = {
)
},
deleteDoc(req, res, next) {
if (next == null) {
next = function (error) {}
}
const { project_id } = req.params
const { doc_id } = req.params
logger.log({ project_id, doc_id }, 'deleting doc')
return DocManager.deleteDoc(project_id, doc_id, function (error) {
if (error != null) {
return next(error)
}
return res.sendStatus(204)
})
},
patchDoc(req, res, next) {
const { project_id, doc_id } = req.params
logger.log({ project_id, doc_id }, 'patching doc')

13
services/docstore/app/js/MongoManager.js
View file

@ -100,19 +100,6 @@ module.exports = MongoManager = {
)
},
markDocAsDeleted(project_id, doc_id, callback) {
db.docs.updateOne(
{
_id: ObjectId(doc_id),
project_id: ObjectId(project_id)
},
{
$set: { deleted: true }
},
callback
)
},
patchDoc(project_id, doc_id, meta, callback) {
db.docs.updateOne(
{

21
services/docstore/test/acceptance/js/DeletingDocsTests.js
View file

@ -203,27 +203,6 @@ 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 () {
deleteTestSuite(DocstoreClient.deleteDoc)

12
services/docstore/test/acceptance/js/helpers/DocstoreClient.js
View file

@ -131,18 +131,6 @@ module.exports = DocstoreClient = {
)
},
deleteDocLegacy(project_id, doc_id, callback) {
if (callback == null) {
callback = function (error, res, body) {}
}
return request.del(
{
url: `http://localhost:${settings.internal.docstore.port}/project/${project_id}/doc/${doc_id}`
},
callback
)
},
deleteDoc(project_id, doc_id, callback) {
DocstoreClient.deleteDocWithDateAndName(
project_id,

166
services/docstore/test/unit/js/DocManagerTests.js
View file

@ -42,51 +42,6 @@ describe('DocManager', function () {
return (this.stubbedError = new Error('blew up'))
})
describe('checkDocExists', function () {
beforeEach(function () {
return (this.DocManager._getDoc = sinon.stub())
})
it('should call get doc with a quick filter', function (done) {
this.DocManager._getDoc.callsArgWith(3, null, { _id: this.doc_id })
return this.DocManager.checkDocExists(
this.project_id,
this.doc_id,
(err, exist) => {
exist.should.equal(true)
this.DocManager._getDoc
.calledWith(this.project_id, this.doc_id, { _id: 1, inS3: true })
.should.equal(true)
return done()
}
)
})
it('should return false when doc is not there', function (done) {
this.DocManager._getDoc.callsArgWith(3, null)
return this.DocManager.checkDocExists(
this.project_id,
this.doc_id,
(err, exist) => {
exist.should.equal(false)
return done()
}
)
})
return it('should return error when get doc errors', function (done) {
this.DocManager._getDoc.callsArgWith(3, 'error')
return this.DocManager.checkDocExists(
this.project_id,
this.doc_id,
(err, exist) => {
err.should.equal('error')
return done()
}
)
})
})
describe('getFullDoc', function () {
beforeEach(function () {
this.DocManager._getDoc = sinon.stub()
@ -414,127 +369,6 @@ describe('DocManager', function () {
})
})
describe('deleteDoc', function () {
describe('when the doc exists', 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().yields(null)
this.DocArchiveManager.archiveDocById = sinon.stub().yields(null)
return this.DocManager.deleteDoc(
this.project_id,
this.doc_id,
this.callback
)
})
it('should get the doc', function () {
return this.DocManager.checkDocExists
.calledWith(this.project_id, this.doc_id)
.should.equal(true)
})
it('should mark doc as deleted', function () {
return this.MongoManager.markDocAsDeleted
.calledWith(this.project_id, this.doc_id)
.should.equal(true)
})
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.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 () {
beforeEach(function () {
this.DocManager.checkDocExists = sinon
.stub()
.callsArgWith(2, null, false)
return this.DocManager.deleteDoc(
this.project_id,
this.doc_id,
this.callback
)
})
return it('should return a NotFoundError', function () {
return this.callback
.calledWith(
sinon.match.has(
'message',
`No such project/doc to delete: ${this.project_id}/${this.doc_id}`
)
)
.should.equal(true)
})
})
})
describe('patchDoc', function () {
describe('when the doc exists', function () {
beforeEach(function () {

21
services/docstore/test/unit/js/HttpControllerTests.js
View file

@ -427,27 +427,6 @@ describe('HttpController', function () {
})
})
describe('deleteDoc', function () {
beforeEach(function () {
this.req.params = {
project_id: this.project_id,
doc_id: this.doc_id
}
this.DocManager.deleteDoc = sinon.stub().callsArg(2)
return this.HttpController.deleteDoc(this.req, this.res, this.next)
})
it('should delete the document', function () {
return this.DocManager.deleteDoc
.calledWith(this.project_id, this.doc_id)
.should.equal(true)
})
return it('should return a 204 (No Content)', function () {
return this.res.sendStatus.calledWith(204).should.equal(true)
})
})
describe('patchDoc', function () {
beforeEach(function () {
this.req.params = {

34
services/docstore/test/unit/js/MongoManagerTests.js
View file

@ -250,40 +250,6 @@ describe('MongoManager', function () {
})
})
describe('markDocAsDeleted', function () {
beforeEach(function () {
this.db.docs.updateOne = sinon.stub().callsArgWith(2, this.stubbedErr)
return (this.oldRev = 77)
})
it('should process the update', function (done) {
return this.MongoManager.markDocAsDeleted(
this.project_id,
this.doc_id,
(err) => {
const args = this.db.docs.updateOne.args[0]
assert.deepEqual(args[0], {
_id: ObjectId(this.doc_id),
project_id: ObjectId(this.project_id)
})
assert.equal(args[1].$set.deleted, true)
return done()
}
)
})
return it('should return the error', function (done) {
return this.MongoManager.markDocAsDeleted(
this.project_id,
this.doc_id,
(err) => {
err.should.equal(this.stubbedErr)
return done()
}
)
})
})
describe('destroyDoc', function () {
beforeEach(function (done) {
this.db.docs.deleteOne = sinon.stub().yields()