diff --git a/services/docstore/app.js b/services/docstore/app.js index 08260dfbba..c6d2df480c 100644 --- a/services/docstore/app.js +++ b/services/docstore/app.js @@ -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) diff --git a/services/docstore/app/js/DocManager.js b/services/docstore/app/js/DocManager.js index 4a97c3688a..39e95f7961 100644 --- a/services/docstore/app/js/DocManager.js +++ b/services/docstore/app/js/DocManager.js @@ -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) => { diff --git a/services/docstore/app/js/HttpController.js b/services/docstore/app/js/HttpController.js index 0d7fc62061..885c241f39 100644 --- a/services/docstore/app/js/HttpController.js +++ b/services/docstore/app/js/HttpController.js @@ -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') diff --git a/services/docstore/app/js/MongoManager.js b/services/docstore/app/js/MongoManager.js index 753abaf1ea..7dc0afeb6c 100644 --- a/services/docstore/app/js/MongoManager.js +++ b/services/docstore/app/js/MongoManager.js @@ -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( { diff --git a/services/docstore/test/acceptance/js/DeletingDocsTests.js b/services/docstore/test/acceptance/js/DeletingDocsTests.js index d481e5795b..0d4e1942dc 100644 --- a/services/docstore/test/acceptance/js/DeletingDocsTests.js +++ b/services/docstore/test/acceptance/js/DeletingDocsTests.js @@ -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) diff --git a/services/docstore/test/acceptance/js/helpers/DocstoreClient.js b/services/docstore/test/acceptance/js/helpers/DocstoreClient.js index fda0a66e1c..390a1a0931 100644 --- a/services/docstore/test/acceptance/js/helpers/DocstoreClient.js +++ b/services/docstore/test/acceptance/js/helpers/DocstoreClient.js @@ -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, diff --git a/services/docstore/test/unit/js/DocManagerTests.js b/services/docstore/test/unit/js/DocManagerTests.js index 624ca641af..1656cd2a6e 100644 --- a/services/docstore/test/unit/js/DocManagerTests.js +++ b/services/docstore/test/unit/js/DocManagerTests.js @@ -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 () { diff --git a/services/docstore/test/unit/js/HttpControllerTests.js b/services/docstore/test/unit/js/HttpControllerTests.js index 219d35119c..d0686cda42 100644 --- a/services/docstore/test/unit/js/HttpControllerTests.js +++ b/services/docstore/test/unit/js/HttpControllerTests.js @@ -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 = { diff --git a/services/docstore/test/unit/js/MongoManagerTests.js b/services/docstore/test/unit/js/MongoManagerTests.js index 2e23087d19..7bf2c2d1a6 100644 --- a/services/docstore/test/unit/js/MongoManagerTests.js +++ b/services/docstore/test/unit/js/MongoManagerTests.js @@ -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()