[misc] drop the deleteDoc endpoint -- use patchDoc instead

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)

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) => {

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')

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(
       {

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)
 

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,

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 () {

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 = {

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()