From dd4f4057f43088328abb3aee31331d3d92f3aabe Mon Sep 17 00:00:00 2001 From: Jakob Ackermann Date: Mon, 15 Feb 2021 13:13:48 +0000 Subject: [PATCH] [misc] add a new endpoint for changing a docs meta data -- incl. deleted - Validate the request payload with joi -- includes acceptance tests. - Reject updates to docs that have been deleted. --- services/docstore/app.js | 20 ++ services/docstore/app/js/DocManager.js | 36 ++++ services/docstore/app/js/Errors.js | 3 + services/docstore/app/js/HttpController.js | 11 + services/docstore/app/js/MongoManager.js | 11 + services/docstore/package-lock.json | 97 ++++++++- services/docstore/package.json | 1 + .../test/acceptance/js/DeletingDocsTests.js | 156 ++++++++++++-- .../acceptance/js/helpers/DocstoreClient.js | 42 +++- .../docstore/test/unit/js/DocManagerTests.js | 193 ++++++++++++++++++ .../test/unit/js/HttpControllerTests.js | 23 +++ .../test/unit/js/MongoManagerTests.js | 27 +++ 12 files changed, 593 insertions(+), 27 deletions(-) diff --git a/services/docstore/app.js b/services/docstore/app.js index 9be08d046c..bd04f6647f 100644 --- a/services/docstore/app.js +++ b/services/docstore/app.js @@ -10,6 +10,11 @@ const Settings = require('settings-sharelatex') const logger = require('logger-sharelatex') const express = require('express') const bodyParser = require('body-parser') +const { + celebrate: validate, + Joi, + errors: handleValidationErrors +} = require('celebrate') const mongodb = require('./app/js/mongodb') const Errors = require('./app/js/Errors') const HttpController = require('./app/js/HttpController') @@ -54,6 +59,18 @@ app.post( bodyParser.json({ limit: (Settings.max_doc_length + 64 * 1024) * 2 }), HttpController.updateDoc ) +app.patch( + '/project/:project_id/doc/:doc_id', + bodyParser.json(), + validate({ + body: { + deleted: Joi.boolean(), + name: Joi.string().when('deleted', { is: true, then: Joi.required() }), + deletedAt: Joi.date().when('deleted', { is: true, then: Joi.required() }) + } + }), + HttpController.patchDoc +) app.delete('/project/:project_id/doc/:doc_id', HttpController.deleteDoc) app.post('/project/:project_id/archive', HttpController.archiveAllDocs) @@ -65,10 +82,13 @@ app.get('/health_check', HttpController.healthCheck) app.get('/status', (req, res) => res.send('docstore is alive')) +app.use(handleValidationErrors()) app.use(function (error, req, res, next) { logger.error({ err: error, req }, 'request errored') if (error instanceof Errors.NotFoundError) { return res.sendStatus(404) + } else if (error instanceof Errors.InvalidOperation) { + return res.status(400).send(error.message) } else { return res.status(500).send('Oops, something went wrong') } diff --git a/services/docstore/app/js/DocManager.js b/services/docstore/app/js/DocManager.js index 409668c313..648bc8d944 100644 --- a/services/docstore/app/js/DocManager.js +++ b/services/docstore/app/js/DocManager.js @@ -320,5 +320,41 @@ module.exports = DocManager = { 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) => { + if (error != null) { + return callback(error) + } + if (!doc) { + return callback( + new Errors.NotFoundError( + `No such project/doc to delete: ${project_id}/${doc_id}` + ) + ) + } + + // deletion is a one-way operation + if (doc.deleted) + return callback( + new Errors.InvalidOperation('Cannot PATCH after doc deletion') + ) + + if (meta.deleted && 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' + ) + } + }) + } + + MongoManager.patchDoc(project_id, doc_id, meta, callback) + }) } } diff --git a/services/docstore/app/js/Errors.js b/services/docstore/app/js/Errors.js index 6a74485494..1b9332b4a0 100644 --- a/services/docstore/app/js/Errors.js +++ b/services/docstore/app/js/Errors.js @@ -4,7 +4,10 @@ const { Errors } = require('@overleaf/object-persistor') class Md5MismatchError extends OError {} +class InvalidOperation extends OError {} + module.exports = { Md5MismatchError, + InvalidOperation, ...Errors } diff --git a/services/docstore/app/js/HttpController.js b/services/docstore/app/js/HttpController.js index 947b83036c..8cb852b4dd 100644 --- a/services/docstore/app/js/HttpController.js +++ b/services/docstore/app/js/HttpController.js @@ -188,6 +188,17 @@ module.exports = HttpController = { }) }, + patchDoc(req, res, next) { + const { project_id, doc_id } = req.params + logger.log({ project_id, doc_id }, 'patching doc') + DocManager.patchDoc(project_id, doc_id, req.body, function (error) { + if (error) { + return next(error) + } + res.sendStatus(204) + }) + }, + _buildDocView(doc) { const doc_view = { _id: doc._id != null ? doc._id.toString() : undefined } for (const attribute of ['lines', 'rev', 'version', 'ranges', 'deleted']) { diff --git a/services/docstore/app/js/MongoManager.js b/services/docstore/app/js/MongoManager.js index c6bd24c639..ea8f7f710b 100644 --- a/services/docstore/app/js/MongoManager.js +++ b/services/docstore/app/js/MongoManager.js @@ -94,6 +94,17 @@ module.exports = MongoManager = { ) }, + patchDoc(project_id, doc_id, meta, callback) { + db.docs.updateOne( + { + _id: ObjectId(doc_id), + project_id: ObjectId(project_id) + }, + { $set: meta }, + callback + ) + }, + markDocAsArchived(doc_id, rev, callback) { const update = { $set: {}, diff --git a/services/docstore/package-lock.json b/services/docstore/package-lock.json index 30c414d75f..f492376dfd 100644 --- a/services/docstore/package-lock.json +++ b/services/docstore/package-lock.json @@ -1001,6 +1001,49 @@ "protobufjs": "^6.8.6" } }, + "@hapi/address": { + "version": "4.1.0", + "resolved": "https://registry.npmjs.org/@hapi/address/-/address-4.1.0.tgz", + "integrity": "sha512-SkszZf13HVgGmChdHo/PxchnSaCJ6cetVqLzyciudzZRT0jcOouIF/Q93mgjw8cce+D+4F4C1Z/WrfFN+O3VHQ==", + "requires": { + "@hapi/hoek": "^9.0.0" + } + }, + "@hapi/formula": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@hapi/formula/-/formula-2.0.0.tgz", + "integrity": "sha512-V87P8fv7PI0LH7LiVi8Lkf3x+KCO7pQozXRssAHNXXL9L1K+uyu4XypLXwxqVDKgyQai6qj3/KteNlrqDx4W5A==" + }, + "@hapi/hoek": { + "version": "9.1.1", + "resolved": "https://registry.npmjs.org/@hapi/hoek/-/hoek-9.1.1.tgz", + "integrity": "sha512-CAEbWH7OIur6jEOzaai83jq3FmKmv4PmX1JYfs9IrYcGEVI/lyL1EXJGCj7eFVJ0bg5QR8LMxBlEtA+xKiLpFw==" + }, + "@hapi/joi": { + "version": "17.1.1", + "resolved": "https://registry.npmjs.org/@hapi/joi/-/joi-17.1.1.tgz", + "integrity": "sha512-p4DKeZAoeZW4g3u7ZeRo+vCDuSDgSvtsB/NpfjXEHTUjSeINAi/RrVOWiVQ1isaoLzMvFEhe8n5065mQq1AdQg==", + "requires": { + "@hapi/address": "^4.0.1", + "@hapi/formula": "^2.0.0", + "@hapi/hoek": "^9.0.0", + "@hapi/pinpoint": "^2.0.0", + "@hapi/topo": "^5.0.0" + } + }, + "@hapi/pinpoint": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@hapi/pinpoint/-/pinpoint-2.0.0.tgz", + "integrity": "sha512-vzXR5MY7n4XeIvLpfl3HtE3coZYO4raKXW766R6DZw/6aLqR26iuZ109K7a0NtF2Db0jxqh7xz2AxkUwpUFybw==" + }, + "@hapi/topo": { + "version": "5.0.0", + "resolved": "https://registry.npmjs.org/@hapi/topo/-/topo-5.0.0.tgz", + "integrity": "sha512-tFJlT47db0kMqVm3H4nQYgn6Pwg10GTZHb1pwmSiv1K4ks6drQOtfEF5ZnPjkvC+y4/bUPHK+bc87QvLcL+WMw==", + "requires": { + "@hapi/hoek": "^9.0.0" + } + }, "@opencensus/core": { "version": "0.0.20", "resolved": "https://registry.npmjs.org/@opencensus/core/-/core-0.0.20.tgz", @@ -1150,6 +1193,24 @@ "resolved": "https://registry.npmjs.org/@protobufjs/utf8/-/utf8-1.1.0.tgz", "integrity": "sha512-Vvn3zZrhQZkkBE8LSuW3em98c0FwgO4nxzv6OdSxPKJIEKY2bGbHn+mhGIPerzI4twdxaP8/0+06HBpwf345Lw==" }, + "@sideway/address": { + "version": "4.1.1", + "resolved": "https://registry.npmjs.org/@sideway/address/-/address-4.1.1.tgz", + "integrity": "sha512-+I5aaQr3m0OAmMr7RQ3fR9zx55sejEYR2BFJaxL+zT3VM2611X0SHvPWIbAUBZVTn/YzYKbV8gJ2oT/QELknfQ==", + "requires": { + "@hapi/hoek": "^9.0.0" + } + }, + "@sideway/formula": { + "version": "3.0.0", + "resolved": "https://registry.npmjs.org/@sideway/formula/-/formula-3.0.0.tgz", + "integrity": "sha512-vHe7wZ4NOXVfkoRb8T5otiENVlT7a3IAiw7H5M2+GO+9CDgcVUUsX1zalAztCmwyOr2RUTGJdgB+ZvSVqmdHmg==" + }, + "@sideway/pinpoint": { + "version": "2.0.0", + "resolved": "https://registry.npmjs.org/@sideway/pinpoint/-/pinpoint-2.0.0.tgz", + "integrity": "sha512-RNiOoTPkptFtSVzQevY/yWtZwf/RxyVnPy/OcA9HBM3MlGDnBEYL5B41H0MTn0Uec8Hi+2qUtTfG2WWZBmMejQ==" + }, "@sinonjs/commons": { "version": "1.8.1", "resolved": "https://registry.npmjs.org/@sinonjs/commons/-/commons-1.8.1.tgz", @@ -1658,7 +1719,7 @@ "bintrees": { "version": "1.0.1", "resolved": "https://registry.npmjs.org/bintrees/-/bintrees-1.0.1.tgz", - "integrity": "sha512-tbaUB1QpTIj4cKY8c1rvNAvEQXA+ekzHmbe4jzNfW3QWsF9GnnP/BRWyl6/qqS53heoYJ93naaFcm/jooONH8g==" + "integrity": "sha1-DmVcm5wkNeqraL9AJyJtK1WjRSQ=" }, "bl": { "version": "2.2.1", @@ -1791,6 +1852,16 @@ "integrity": "sha512-4tYFyifaFfGacoiObjJegolkwSU4xQNGbVgUiNYVUxbQ2x2lUsFvY4hVgVzGiIe6WLOPqycWXA40l+PWsxthUw==", "dev": true }, + "celebrate": { + "version": "13.0.4", + "resolved": "https://registry.npmjs.org/celebrate/-/celebrate-13.0.4.tgz", + "integrity": "sha512-gUtAjEtFyY9PvuuQJq1uyuF46gLetVZzyUKXBDBqqvgzCjTSfwXP8L+WcGt1NrLQvUxXdlzhFolW2Bt9DDEV+g==", + "requires": { + "escape-html": "1.0.3", + "joi": "17.x.x", + "lodash": "4.17.x" + } + }, "chai": { "version": "4.2.0", "resolved": "https://registry.npmjs.org/chai/-/chai-4.2.0.tgz", @@ -3061,7 +3132,7 @@ "findit2": { "version": "2.2.3", "resolved": "https://registry.npmjs.org/findit2/-/findit2-2.2.3.tgz", - "integrity": "sha512-lg/Moejf4qXovVutL0Lz4IsaPoNYMuxt4PA0nGqFxnJ1CTTGGlEO2wKgoDpwknhvZ8k4Q2F+eesgkLbG2Mxfog==" + "integrity": "sha1-WKRmaX34piBc39vzlVNri9d3pfY=" }, "flat": { "version": "4.1.0", @@ -3957,6 +4028,18 @@ "resolved": "https://registry.npmjs.org/jmespath/-/jmespath-0.15.0.tgz", "integrity": "sha1-o/Iiqarp+Wb10nx5ZRDigJF2Qhc=" }, + "joi": { + "version": "17.4.0", + "resolved": "https://registry.npmjs.org/joi/-/joi-17.4.0.tgz", + "integrity": "sha512-F4WiW2xaV6wc1jxete70Rw4V/VuMd6IN+a5ilZsxG4uYtUXWu2kq9W5P2dz30e7Gmw8RCbY/u/uk+dMPma9tAg==", + "requires": { + "@hapi/hoek": "^9.0.0", + "@hapi/topo": "^5.0.0", + "@sideway/address": "^4.1.0", + "@sideway/formula": "^3.0.0", + "@sideway/pinpoint": "^2.0.0" + } + }, "js-tokens": { "version": "4.0.0", "resolved": "https://registry.npmjs.org/js-tokens/-/js-tokens-4.0.0.tgz", @@ -4589,7 +4672,7 @@ "module-details-from-path": { "version": "1.0.3", "resolved": "https://registry.npmjs.org/module-details-from-path/-/module-details-from-path-1.0.3.tgz", - "integrity": "sha512-ySViT69/76t8VhE1xXHK6Ch4NcDd26gx0MzKXLO+F7NOtnqH68d9zF94nT8ZWSxXh8ELOERsnJO/sWt1xZYw5A==" + "integrity": "sha1-EUyUlnPiqKNenTV4hSeqN7Z52is=" }, "moment": { "version": "2.24.0", @@ -6208,7 +6291,7 @@ "resolve-from": { "version": "2.0.0", "resolved": "https://registry.npmjs.org/resolve-from/-/resolve-from-2.0.0.tgz", - "integrity": "sha512-qpFcKaXsq8+oRoLilkwyc7zHGF5i9Q2/25NIgLQQ/+VVv9rU4qvr6nXVAw1DsnXJyQkZsR4Ytfbtg5ehfcUssQ==" + "integrity": "sha1-lICrIOlP+h2egKgEx+oUdhGWa1c=" }, "restore-cursor": { "version": "3.1.0", @@ -6513,7 +6596,7 @@ "sparse-bitfield": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/sparse-bitfield/-/sparse-bitfield-3.0.3.tgz", - "integrity": "sha512-kvzhi7vqKTfkh0PZU+2D2PIllw2ymqJKujUcyPMd9Y75Nv4nPbGJZXNhxsgdQab2BmlDct1YnfQCguEvHr7VsQ==", + "integrity": "sha1-/0rm5oZWBWuks+eSqzM004JzyhE=", "optional": true, "requires": { "memory-pager": "^1.0.2" @@ -6741,7 +6824,7 @@ "stubs": { "version": "3.0.0", "resolved": "https://registry.npmjs.org/stubs/-/stubs-3.0.0.tgz", - "integrity": "sha512-PdHt7hHUJKxvTCgbKX9C1V/ftOcjJQgz8BZwNfV5c4B6dcGqlpelTbJ999jBGZ2jYiPAwcX5dP6oBwVlBlUbxw==" + "integrity": "sha1-6NK6H6nJBXAwPAMLaQD31fiavls=" }, "supports-color": { "version": "6.0.0", @@ -6824,7 +6907,7 @@ "tdigest": { "version": "0.1.1", "resolved": "https://registry.npmjs.org/tdigest/-/tdigest-0.1.1.tgz", - "integrity": "sha512-CXcDY/NIgIbKZPx5H4JJNpq6JwJhU5Z4+yWj4ZghDc7/9nVajiRlPPyMXRePPPlBfcayUqtoCXjo7/Hm82ecUA==", + "integrity": "sha1-Ljyyw56kSeVdHmzZEReszKRYgCE=", "requires": { "bintrees": "1.0.1" } diff --git a/services/docstore/package.json b/services/docstore/package.json index 1d794d1398..06e5fa6fc3 100644 --- a/services/docstore/package.json +++ b/services/docstore/package.json @@ -23,6 +23,7 @@ "@overleaf/object-persistor": "https://github.com/overleaf/object-persistor/archive/4ca62157a2beb747e9a56da3ce1569124b90378a.tar.gz", "async": "^2.6.3", "body-parser": "^1.19.0", + "celebrate": "^13.0.4", "express": "^4.17.1", "logger-sharelatex": "^2.2.0", "mongodb": "^3.6.0", diff --git a/services/docstore/test/acceptance/js/DeletingDocsTests.js b/services/docstore/test/acceptance/js/DeletingDocsTests.js index d32cd52b6b..3872a62762 100644 --- a/services/docstore/test/acceptance/js/DeletingDocsTests.js +++ b/services/docstore/test/acceptance/js/DeletingDocsTests.js @@ -21,7 +21,7 @@ const Settings = require('settings-sharelatex') const DocstoreClient = require('./helpers/DocstoreClient') -describe('Deleting a doc', function () { +function deleteTestSuite(deleteDoc) { beforeEach(function (done) { this.project_id = ObjectId() this.doc_id = ObjectId() @@ -60,14 +60,10 @@ describe('Deleting a doc', function () { describe('when the doc exists', function () { beforeEach(function (done) { - return DocstoreClient.deleteDoc( - this.project_id, - this.doc_id, - (error, res, doc) => { - this.res = res - return done() - } - ) + deleteDoc(this.project_id, this.doc_id, (error, res, doc) => { + this.res = res + return done() + }) }) afterEach(function (done) { @@ -108,16 +104,16 @@ describe('Deleting a doc', function () { describe('when archiveOnSoftDelete is enabled', function () { let archiveOnSoftDelete - beforeEach(function overwriteSetting() { + beforeEach('overwrite settings', function () { archiveOnSoftDelete = Settings.docstore.archiveOnSoftDelete Settings.docstore.archiveOnSoftDelete = true }) - afterEach(function restoreSetting() { + afterEach('restore settings', function () { Settings.docstore.archiveOnSoftDelete = archiveOnSoftDelete }) - beforeEach(function deleteDoc(done) { - DocstoreClient.deleteDoc(this.project_id, this.doc_id, (error, res) => { + beforeEach('delete Doc', function (done) { + deleteDoc(this.project_id, this.doc_id, (error, res) => { this.res = res done() }) @@ -177,7 +173,7 @@ describe('Deleting a doc', function () { }) it('should return a 404 when trying to delete', function (done) { - DocstoreClient.deleteDoc(otherProjectId, this.doc_id, (error, res) => { + deleteDoc(otherProjectId, this.doc_id, (error, res) => { if (error) return done(error) expect(res.statusCode).to.equal(404) done() @@ -201,15 +197,137 @@ describe('Deleting a doc', function () { return it('should return a 404', function (done) { const missing_doc_id = ObjectId() - return DocstoreClient.deleteDoc( + deleteDoc(this.project_id, missing_doc_id, (error, res, doc) => { + res.statusCode.should.equal(404) + return done() + }) + }) + }) +} + +describe('Delete via DELETE', function () { + deleteTestSuite(DocstoreClient.deleteDocLegacy) +}) + +describe('Delete via PATCH', function () { + deleteTestSuite(DocstoreClient.deleteDoc) + + describe('deleting a doc twice', function () { + beforeEach('perform 1st DELETE request', function (done) { + DocstoreClient.deleteDoc(this.project_id, this.doc_id, done) + }) + + beforeEach('get doc before 2nd DELETE request', function (done) { + db.docs.find({ _id: this.doc_id }).toArray((error, docs) => { + if (error) return done(error) + this.docBefore = docs[0] + if (!this.docBefore) return done(new Error('doc not found')) + done() + }) + }) + + beforeEach('perform 2nd DELETE request', function (done) { + DocstoreClient.deleteDoc(this.project_id, this.doc_id, (error, res) => { + this.res1 = res + done(error) + }) + }) + + it('should reject the 2nd request', function () { + expect(this.res1.statusCode).to.equal(400) + }) + + it('should not alter the previous doc state', function (done) { + db.docs.find({ _id: this.doc_id }).toArray((error, docs) => { + if (error) return done(error) + const docAfter = docs[0] + if (!docAfter) return done(new Error('doc not found')) + + expect(docAfter).to.deep.equal(this.docBefore) + done() + }) + }) + }) + + describe('when providing a custom doc name in the delete request', function () { + beforeEach(function (done) { + DocstoreClient.deleteDocWithName( this.project_id, - missing_doc_id, - (error, res, doc) => { - res.statusCode.should.equal(404) - return done() + this.doc_id, + 'wombat.tex', + done + ) + }) + + it('should insert the doc name into the docs collection', function (done) { + db.docs.find({ _id: this.doc_id }).toArray((error, docs) => { + if (error) return done(error) + expect(docs[0].name).to.equal('wombat.tex') + done() + }) + }) + }) + + describe('when providing a custom deletedAt date in the delete request', function () { + beforeEach('record date and delay', function (done) { + this.deletedAt = new Date() + setTimeout(done, 5) + }) + + beforeEach('perform deletion with past date', function (done) { + DocstoreClient.deleteDocWithDate( + this.project_id, + this.doc_id, + this.deletedAt, + done + ) + }) + + it('should insert the date into the docs collection', function (done) { + db.docs.find({ _id: this.doc_id }).toArray((error, docs) => { + if (error) return done(error) + expect(docs[0].deletedAt.toISOString()).to.equal( + this.deletedAt.toISOString() + ) + done() + }) + }) + }) + + describe('when providing no doc name in the delete request', function () { + beforeEach(function (done) { + DocstoreClient.deleteDocWithName( + this.project_id, + this.doc_id, + '', + (error, res) => { + this.res = res + done(error) } ) }) + + it('should reject the request', function () { + expect(this.res.statusCode).to.equal(400) + }) + }) + + describe('when providing no date in the delete request', function () { + beforeEach(function (done) { + DocstoreClient.deleteDocWithDate( + this.project_id, + this.doc_id, + '', + (error, res) => { + this.res = res + done(error) + } + ) + }) + + it('should reject the request', function () { + expect(this.res.statusCode).to.equal(400) + }) }) }) diff --git a/services/docstore/test/acceptance/js/helpers/DocstoreClient.js b/services/docstore/test/acceptance/js/helpers/DocstoreClient.js index ee4f26520e..7248fff744 100644 --- a/services/docstore/test/acceptance/js/helpers/DocstoreClient.js +++ b/services/docstore/test/acceptance/js/helpers/DocstoreClient.js @@ -115,7 +115,7 @@ module.exports = DocstoreClient = { ) }, - deleteDoc(project_id, doc_id, callback) { + deleteDocLegacy(project_id, doc_id, callback) { if (callback == null) { callback = function (error, res, body) {} } @@ -127,6 +127,46 @@ module.exports = DocstoreClient = { ) }, + deleteDoc(project_id, doc_id, callback) { + DocstoreClient.deleteDocWithDateAndName( + project_id, + doc_id, + new Date(), + 'main.tex', + callback + ) + }, + + deleteDocWithDate(project_id, doc_id, date, callback) { + DocstoreClient.deleteDocWithDateAndName( + project_id, + doc_id, + date, + 'main.tex', + callback + ) + }, + + deleteDocWithName(project_id, doc_id, name, callback) { + DocstoreClient.deleteDocWithDateAndName( + project_id, + doc_id, + new Date(), + name, + callback + ) + }, + + deleteDocWithDateAndName(project_id, doc_id, deletedAt, name, callback) { + request.patch( + { + url: `http://localhost:${settings.internal.docstore.port}/project/${project_id}/doc/${doc_id}`, + json: { name, deleted: true, deletedAt } + }, + callback + ) + }, + archiveAllDoc(project_id, callback) { if (callback == null) { callback = function (error, res, body) {} diff --git a/services/docstore/test/unit/js/DocManagerTests.js b/services/docstore/test/unit/js/DocManagerTests.js index 597129b1bd..d581ce743b 100644 --- a/services/docstore/test/unit/js/DocManagerTests.js +++ b/services/docstore/test/unit/js/DocManagerTests.js @@ -545,6 +545,199 @@ describe('DocManager', function () { }) }) + describe('patchDoc', function () { + describe('when the doc exists', function () { + beforeEach(function () { + this.lines = ['mock', 'doc', 'lines'] + this.rev = 77 + this.MongoManager.findDoc = sinon + .stub() + .yields(null, { _id: ObjectId(this.doc_id) }) + this.MongoManager.patchDoc = sinon.stub().yields(null) + this.DocArchiveManager.archiveDocById = sinon.stub().yields(null) + this.meta = {} + }) + + describe('standard path', function () { + beforeEach(function (done) { + this.callback = sinon.stub().callsFake(done) + this.DocManager.patchDoc( + this.project_id, + this.doc_id, + this.meta, + this.callback + ) + }) + + it('should get the doc', function () { + expect(this.MongoManager.findDoc).to.have.been.calledWith( + this.project_id, + this.doc_id + ) + }) + + it('should persist the meta', function () { + expect(this.MongoManager.patchDoc).to.have.been.calledWith( + this.project_id, + this.doc_id, + this.meta + ) + }) + + it('should return the callback', function () { + expect(this.callback).to.have.been.calledWith(null) + }) + }) + + describe('background flush disabled and deleting a doc', function () { + beforeEach(function (done) { + this.settings.docstore.archiveOnSoftDelete = false + this.meta.deleted = true + + this.callback = sinon.stub().callsFake(done) + this.DocManager.patchDoc( + this.project_id, + this.doc_id, + this.meta, + this.callback + ) + }) + + it('should not flush the doc out of mongo', function () { + expect(this.DocArchiveManager.archiveDocById).to.not.have.been.called + }) + }) + + describe('background flush enabled and not deleting a doc', function () { + beforeEach(function (done) { + this.settings.docstore.archiveOnSoftDelete = false + this.meta.deleted = false + this.callback = sinon.stub().callsFake(done) + this.DocManager.patchDoc( + this.project_id, + this.doc_id, + this.meta, + this.callback + ) + }) + + it('should not flush the doc out of mongo', function () { + expect(this.DocArchiveManager.archiveDocById).to.not.have.been.called + }) + }) + + describe('background flush enabled and deleting a doc', function () { + beforeEach(function () { + this.settings.docstore.archiveOnSoftDelete = true + this.meta.deleted = true + this.logger.warn = sinon.stub() + }) + + describe('when the background flush succeeds', function () { + beforeEach(function (done) { + this.DocArchiveManager.archiveDocById = sinon.stub().yields(null) + this.callback = sinon.stub().callsFake(done) + this.DocManager.patchDoc( + this.project_id, + this.doc_id, + this.meta, + this.callback + ) + }) + + it('should not log a warning', function () { + expect(this.logger.warn).to.not.have.been.called + }) + + it('should flush the doc out of mongo', function () { + expect( + this.DocArchiveManager.archiveDocById + ).to.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.DocArchiveManager.archiveDocById = sinon + .stub() + .yields(this.err) + this.callback = sinon.stub().callsFake(done) + this.DocManager.patchDoc( + this.project_id, + this.doc_id, + this.meta, + this.callback + ) + }) + + it('should log a warning', function () { + expect(this.logger.warn).to.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 () { + expect(this.callback).to.have.been.calledWith(null) + }) + }) + }) + }) + + describe('when the doc is already deleted', function () { + beforeEach(function (done) { + this.MongoManager.findDoc = sinon + .stub() + .yields(null, { _id: ObjectId(this.doc_id), deleted: true }) + this.MongoManager.patchDoc = sinon.stub() + + this.callback = sinon.stub().callsFake(() => done()) + this.DocManager.patchDoc( + this.project_id, + this.doc_id, + 'tomato.tex', + this.callback + ) + }) + + it('should reject the operation', function () { + expect(this.callback).to.have.been.calledWith( + sinon.match.has('message', 'Cannot PATCH after doc deletion') + ) + }) + + it('should not persist the change to mongo', function () { + expect(this.MongoManager.patchDoc).to.not.have.been.called + }) + }) + + describe('when the doc does not exist', function () { + beforeEach(function () { + this.MongoManager.findDoc = sinon.stub().yields(null) + this.DocManager.patchDoc( + this.project_id, + this.doc_id, + {}, + this.callback + ) + }) + + it('should return a NotFoundError', function () { + expect(this.callback).to.have.been.calledWith( + sinon.match.has( + 'message', + `No such project/doc to delete: ${this.project_id}/${this.doc_id}` + ) + ) + }) + }) + }) + return describe('updateDoc', function () { beforeEach(function () { this.oldDocLines = ['old', 'doc', 'lines'] diff --git a/services/docstore/test/unit/js/HttpControllerTests.js b/services/docstore/test/unit/js/HttpControllerTests.js index 6638dbdbf8..16d4c7f584 100644 --- a/services/docstore/test/unit/js/HttpControllerTests.js +++ b/services/docstore/test/unit/js/HttpControllerTests.js @@ -456,6 +456,29 @@ describe('HttpController', function () { }) }) + describe('patchDoc', function () { + beforeEach(function () { + this.req.params = { + project_id: this.project_id, + doc_id: this.doc_id + } + this.req.body = { name: 'foo.tex' } + this.DocManager.patchDoc = sinon.stub().yields(null) + this.HttpController.patchDoc(this.req, this.res, this.next) + }) + + it('should delete the document', function () { + expect(this.DocManager.patchDoc).to.have.been.calledWith( + this.project_id, + this.doc_id + ) + }) + + it('should return a 204 (No Content)', function () { + expect(this.res.sendStatus).to.have.been.calledWith(204) + }) + }) + describe('archiveAllDocs', function () { beforeEach(function () { this.req.params = { project_id: this.project_id } diff --git a/services/docstore/test/unit/js/MongoManagerTests.js b/services/docstore/test/unit/js/MongoManagerTests.js index 12fd3fba6c..eca77a55d9 100644 --- a/services/docstore/test/unit/js/MongoManagerTests.js +++ b/services/docstore/test/unit/js/MongoManagerTests.js @@ -72,6 +72,33 @@ describe('MongoManager', function () { }) }) + describe('patchDoc', function () { + beforeEach(function (done) { + this.db.docs.updateOne = sinon.stub().yields(null) + this.meta = { name: 'foo.tex' } + this.callback.callsFake(done) + this.MongoManager.patchDoc( + this.project_id, + this.doc_id, + this.meta, + this.callback + ) + }) + + it('should pass the parameter along', function () { + this.db.docs.updateOne.should.have.been.calledWith( + { + _id: ObjectId(this.doc_id), + project_id: ObjectId(this.project_id) + }, + { + $set: this.meta + }, + this.callback + ) + }) + }) + describe('getProjectsDocs', function () { beforeEach(function () { this.filter = { lines: true }