From 0fd24b5133f74473efc912b2d13ce3b28434117e Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Fri, 30 Jul 2021 16:13:48 +0100 Subject: [PATCH 1/2] peek at docs without fetching from mongo --- services/document-updater/app.js | 1 + .../document-updater/app/js/HttpController.js | 18 ++++++++++++++++++ .../js/HttpController/HttpControllerTests.js | 1 + 3 files changed, 20 insertions(+) diff --git a/services/document-updater/app.js b/services/document-updater/app.js index c724b74d33..61f254d7f2 100644 --- a/services/document-updater/app.js +++ b/services/document-updater/app.js @@ -53,6 +53,7 @@ app.param('doc_id', (req, res, next, docId) => { }) app.get('/project/:project_id/doc/:doc_id', HttpController.getDoc) +app.get('/project/:project_id/doc/:doc_id/peek', HttpController.peekDoc) // temporarily keep the GET method for backwards compatibility app.get('/project/:project_id/doc', HttpController.getProjectDocsAndFlushIfOld) // will migrate to the POST method of get_and_flush_if_old instead diff --git a/services/document-updater/app/js/HttpController.js b/services/document-updater/app/js/HttpController.js index 6bffb6ec4a..4ea7a00d4c 100644 --- a/services/document-updater/app/js/HttpController.js +++ b/services/document-updater/app/js/HttpController.js @@ -1,6 +1,7 @@ const DocumentManager = require('./DocumentManager') const HistoryManager = require('./HistoryManager') const ProjectManager = require('./ProjectManager') +const RedisManager = require('./RedisManager') const Errors = require('./Errors') const logger = require('logger-sharelatex') const Settings = require('@overleaf/settings') @@ -11,6 +12,7 @@ const async = require('async') module.exports = { getDoc, + peekDoc, getProjectDocsAndFlushIfOld, clearProjectState, setDoc, @@ -65,6 +67,22 @@ function getDoc(req, res, next) { ) } +// return the doc from redis if present, but don't load it from mongo +function peekDoc(req, res, next) { + const docId = req.params.doc_id + const projectId = req.params.project_id + logger.log({ projectId, docId }, 'peeking at doc via http') + RedisManager.getDoc(projectId, docId, function (error, lines, version) { + if (error) { + return next(error) + } + if (lines == null || version == null) { + return next(new Errors.NotFoundError('document not found')) + } + res.json({ id: docId, lines, version }) + }) +} + function _getTotalSizeOfLines(lines) { let size = 0 for (const line of lines) { diff --git a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js index 3a926d5e0c..7bea76edd0 100644 --- a/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js +++ b/services/document-updater/test/unit/js/HttpController/HttpControllerTests.js @@ -14,6 +14,7 @@ describe('HttpController', function () { './ProjectManager': (this.ProjectManager = {}), './ProjectFlusher': { flushAllProjects() {} }, './DeleteQueueManager': (this.DeleteQueueManager = {}), + './RedisManager': (this.RedisManager = {}), './Metrics': (this.Metrics = {}), './Errors': Errors, }, From 5e2d4d21698fb3712631278fd898f4cd89d3be2a Mon Sep 17 00:00:00 2001 From: Brian Gough Date: Mon, 2 Aug 2021 14:13:38 +0100 Subject: [PATCH 2/2] add acceptance tests for peek --- .../test/acceptance/js/PeekingADoc.js | 99 +++++++++++++++++++ .../acceptance/js/helpers/DocUpdaterClient.js | 12 +++ 2 files changed, 111 insertions(+) create mode 100644 services/document-updater/test/acceptance/js/PeekingADoc.js diff --git a/services/document-updater/test/acceptance/js/PeekingADoc.js b/services/document-updater/test/acceptance/js/PeekingADoc.js new file mode 100644 index 0000000000..43e463ca51 --- /dev/null +++ b/services/document-updater/test/acceptance/js/PeekingADoc.js @@ -0,0 +1,99 @@ +const sinon = require('sinon') +const MockWebApi = require('./helpers/MockWebApi') +const DocUpdaterClient = require('./helpers/DocUpdaterClient') +const DocUpdaterApp = require('./helpers/DocUpdaterApp') + +describe('Peeking a document', function () { + before(function (done) { + this.lines = ['one', 'two', 'three'] + this.version = 42 + return DocUpdaterApp.ensureRunning(done) + }) + + describe('when the document is not loaded', function () { + before(function (done) { + this.project_id = DocUpdaterClient.randomId() + this.doc_id = DocUpdaterClient.randomId() + sinon.spy(MockWebApi, 'getDocument') + + MockWebApi.insertDoc(this.project_id, this.doc_id, { + lines: this.lines, + version: this.version, + }) + + return DocUpdaterClient.peekDoc( + this.project_id, + this.doc_id, + (error, res, returnedDoc) => { + this.error = error + this.res = res + this.returnedDoc = returnedDoc + return done() + } + ) + }) + + after(function () { + return MockWebApi.getDocument.restore() + }) + + it('should return a 404 response', function () { + this.res.statusCode.should.equal(404) + }) + + it('should not load the document from the web API', function () { + return MockWebApi.getDocument.called.should.equal(false) + }) + }) + + describe('when the document is already loaded', function () { + before(function (done) { + this.project_id = DocUpdaterClient.randomId() + this.doc_id = DocUpdaterClient.randomId() + + MockWebApi.insertDoc(this.project_id, this.doc_id, { + lines: this.lines, + version: this.version, + }) + return DocUpdaterClient.preloadDoc( + this.project_id, + this.doc_id, + error => { + if (error != null) { + throw error + } + sinon.spy(MockWebApi, 'getDocument') + return DocUpdaterClient.getDoc( + this.project_id, + this.doc_id, + (error, res, returnedDoc) => { + this.res = res + this.returnedDoc = returnedDoc + return done() + } + ) + } + ) + }) + + after(function () { + return MockWebApi.getDocument.restore() + }) + + it('should return a 200 response', function () { + this.res.statusCode.should.equal(200) + }) + + it('should return the document lines', function () { + return this.returnedDoc.lines.should.deep.equal(this.lines) + }) + + it('should return the document version', function () { + return this.returnedDoc.version.should.equal(this.version) + }) + + it('should not load the document from the web API', function () { + return MockWebApi.getDocument.called.should.equal(false) + }) + }) +}) diff --git a/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js b/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js index 71e7915c0f..9a3234628c 100644 --- a/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js +++ b/services/document-updater/test/acceptance/js/helpers/DocUpdaterClient.js @@ -123,6 +123,18 @@ module.exports = DocUpdaterClient = { DocUpdaterClient.getDoc(projectId, docId, callback) }, + peekDoc(projectId, docId, callback) { + request.get( + `http://localhost:3003/project/${projectId}/doc/${docId}/peek`, + (error, res, body) => { + if (body != null && res.statusCode >= 200 && res.statusCode < 300) { + body = JSON.parse(body) + } + callback(error, res, body) + } + ) + }, + flushDoc(projectId, docId, callback) { request.post( `http://localhost:3003/project/${projectId}/doc/${docId}/flush`,