From 9f4541c266d3d3ce1baed91cde01f9cc42ba5b14 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Thu, 19 Aug 2021 10:40:31 -0400 Subject: [PATCH] Merge pull request #4761 from overleaf/em-peek-doc Do not unarchive docs when resyncing project history GitOrigin-RevId: c7df75789c01e6c85b464a9b94b14654d8568407 --- .../app/js/DocumentManager.js | 1 + .../app/js/PersistenceManager.js | 308 +++++----- .../DocumentManager/DocumentManagerTests.js | 337 +++++------ .../PersistenceManagerTests.js | 187 +++--- .../src/Features/Docstore/DocstoreManager.js | 538 +++++++++--------- .../Features/Documents/DocumentController.js | 229 ++++---- .../unit/src/Docstore/DocstoreManagerTests.js | 178 +++--- .../src/Documents/DocumentControllerTests.js | 111 +--- 8 files changed, 849 insertions(+), 1040 deletions(-) diff --git a/services/document-updater/app/js/DocumentManager.js b/services/document-updater/app/js/DocumentManager.js index a79d4aa187..b1384f642a 100644 --- a/services/document-updater/app/js/DocumentManager.js +++ b/services/document-updater/app/js/DocumentManager.js @@ -600,6 +600,7 @@ module.exports = DocumentManager = { return PersistenceManager.getDoc( project_id, doc_id, + { peek: true }, function ( error, lines, diff --git a/services/document-updater/app/js/PersistenceManager.js b/services/document-updater/app/js/PersistenceManager.js index d7df831683..06aa050ba0 100644 --- a/services/document-updater/app/js/PersistenceManager.js +++ b/services/document-updater/app/js/PersistenceManager.js @@ -1,20 +1,3 @@ -/* eslint-disable - camelcase, - handle-callback-err, - no-unsafe-negation, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS205: Consider reworking code to avoid use of IIFEs - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ -let PersistenceManager const Settings = require('@overleaf/settings') const Errors = require('./Errors') const Metrics = require('./Metrics') @@ -29,172 +12,149 @@ const request = require('requestretry').defaults({ // hold us up, and need to bail out quickly if there is a problem. const MAX_HTTP_REQUEST_LENGTH = 5000 // 5 seconds -const updateMetric = function (method, error, response) { +function updateMetric(method, error, response) { // find the status, with special handling for connection timeouts // https://github.com/request/request#timeouts - const status = (() => { - if ((error != null ? error.connect : undefined) === true) { - return `${error.code} (connect)` - } else if (error != null) { - return error.code - } else if (response != null) { - return response.statusCode - } - })() + let status + if (error && error.connect === true) { + status = `${error.code} (connect)` + } else if (error) { + status = error.code + } else if (response) { + status = response.statusCode + } + Metrics.inc(method, 1, { status }) - if ((error != null ? error.attempts : undefined) > 1) { + if (error && error.attempts > 1) { Metrics.inc(`${method}-retries`, 1, { status: 'error' }) } - if ((response != null ? response.attempts : undefined) > 1) { - return Metrics.inc(`${method}-retries`, 1, { status: 'success' }) + if (response && response.attempts > 1) { + Metrics.inc(`${method}-retries`, 1, { status: 'success' }) } } -module.exports = PersistenceManager = { - getDoc(project_id, doc_id, _callback) { - if (_callback == null) { - _callback = function ( - error, - lines, - version, - ranges, - pathname, - projectHistoryId, - projectHistoryType - ) {} - } - const timer = new Metrics.Timer('persistenceManager.getDoc') - const callback = function (...args) { - timer.done() - return _callback(...Array.from(args || [])) - } +function getDoc(projectId, docId, options = {}, _callback) { + const timer = new Metrics.Timer('persistenceManager.getDoc') + if (typeof options === 'function') { + _callback = options + options = {} + } + const callback = function (...args) { + timer.done() + _callback(...args) + } - const urlPath = `/project/${project_id}/doc/${doc_id}` - return request( - { - url: `${Settings.apis.web.url}${urlPath}`, - method: 'GET', - headers: { - accept: 'application/json', - }, - auth: { - user: Settings.apis.web.user, - pass: Settings.apis.web.pass, - sendImmediately: true, - }, - jar: false, - timeout: MAX_HTTP_REQUEST_LENGTH, - }, - function (error, res, body) { - updateMetric('getDoc', error, res) - if (error != null) { - logger.error( - { err: error, project_id, doc_id }, - 'web API request failed' - ) - return callback(new Error('error connecting to web API')) - } - if (res.statusCode >= 200 && res.statusCode < 300) { - try { - body = JSON.parse(body) - } catch (e) { - return callback(e) - } - if (body.lines == null) { - return callback(new Error('web API response had no doc lines')) - } - if (body.version == null || !body.version instanceof Number) { - return callback( - new Error('web API response had no valid doc version') - ) - } - if (body.pathname == null) { - return callback( - new Error('web API response had no valid doc pathname') - ) - } - return callback( - null, - body.lines, - body.version, - body.ranges, - body.pathname, - body.projectHistoryId, - body.projectHistoryType - ) - } else if (res.statusCode === 404) { - return callback( - new Errors.NotFoundError(`doc not not found: ${urlPath}`) - ) - } else { - return callback( - new Error(`error accessing web API: ${urlPath} ${res.statusCode}`) - ) - } + const urlPath = `/project/${projectId}/doc/${docId}` + const requestParams = { + url: `${Settings.apis.web.url}${urlPath}`, + method: 'GET', + headers: { + accept: 'application/json', + }, + auth: { + user: Settings.apis.web.user, + pass: Settings.apis.web.pass, + sendImmediately: true, + }, + jar: false, + timeout: MAX_HTTP_REQUEST_LENGTH, + } + if (options.peek) { + requestParams.qs = { peek: 'true' } + } + request(requestParams, (error, res, body) => { + updateMetric('getDoc', error, res) + if (error) { + logger.error({ err: error, projectId, docId }, 'web API request failed') + return callback(new Error('error connecting to web API')) + } + if (res.statusCode >= 200 && res.statusCode < 300) { + try { + body = JSON.parse(body) + } catch (e) { + return callback(e) } - ) - }, - - setDoc( - project_id, - doc_id, - lines, - version, - ranges, - lastUpdatedAt, - lastUpdatedBy, - _callback - ) { - if (_callback == null) { - _callback = function (error) {} - } - const timer = new Metrics.Timer('persistenceManager.setDoc') - const callback = function (...args) { - timer.done() - return _callback(...Array.from(args || [])) - } - - const urlPath = `/project/${project_id}/doc/${doc_id}` - return request( - { - url: `${Settings.apis.web.url}${urlPath}`, - method: 'POST', - json: { - lines, - ranges, - version, - lastUpdatedBy, - lastUpdatedAt, - }, - auth: { - user: Settings.apis.web.user, - pass: Settings.apis.web.pass, - sendImmediately: true, - }, - jar: false, - timeout: MAX_HTTP_REQUEST_LENGTH, - }, - function (error, res, body) { - updateMetric('setDoc', error, res) - if (error != null) { - logger.error( - { err: error, project_id, doc_id }, - 'web API request failed' - ) - return callback(new Error('error connecting to web API')) - } - if (res.statusCode >= 200 && res.statusCode < 300) { - return callback(null) - } else if (res.statusCode === 404) { - return callback( - new Errors.NotFoundError(`doc not not found: ${urlPath}`) - ) - } else { - return callback( - new Error(`error accessing web API: ${urlPath} ${res.statusCode}`) - ) - } + if (body.lines == null) { + return callback(new Error('web API response had no doc lines')) } - ) - }, + if (body.version == null) { + return callback(new Error('web API response had no valid doc version')) + } + if (body.pathname == null) { + return callback(new Error('web API response had no valid doc pathname')) + } + callback( + null, + body.lines, + body.version, + body.ranges, + body.pathname, + body.projectHistoryId, + body.projectHistoryType + ) + } else if (res.statusCode === 404) { + callback(new Errors.NotFoundError(`doc not not found: ${urlPath}`)) + } else { + callback( + new Error(`error accessing web API: ${urlPath} ${res.statusCode}`) + ) + } + }) } + +function setDoc( + projectId, + docId, + lines, + version, + ranges, + lastUpdatedAt, + lastUpdatedBy, + _callback +) { + const timer = new Metrics.Timer('persistenceManager.setDoc') + const callback = function (...args) { + timer.done() + _callback(...args) + } + + const urlPath = `/project/${projectId}/doc/${docId}` + request( + { + url: `${Settings.apis.web.url}${urlPath}`, + method: 'POST', + json: { + lines, + ranges, + version, + lastUpdatedBy, + lastUpdatedAt, + }, + auth: { + user: Settings.apis.web.user, + pass: Settings.apis.web.pass, + sendImmediately: true, + }, + jar: false, + timeout: MAX_HTTP_REQUEST_LENGTH, + }, + (error, res, body) => { + updateMetric('setDoc', error, res) + if (error) { + logger.error({ err: error, projectId, docId }, 'web API request failed') + return callback(new Error('error connecting to web API')) + } + if (res.statusCode >= 200 && res.statusCode < 300) { + callback(null) + } else if (res.statusCode === 404) { + callback(new Errors.NotFoundError(`doc not not found: ${urlPath}`)) + } else { + callback( + new Error(`error accessing web API: ${urlPath} ${res.statusCode}`) + ) + } + } + ) +} + +module.exports = { getDoc, setDoc } diff --git a/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js b/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js index bac5ae6a85..9a3aa95c5e 100644 --- a/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js +++ b/services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js @@ -1,17 +1,3 @@ -/* eslint-disable - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS101: Remove unnecessary use of Array.from - * DS102: Remove unnecessary code created because of implicit returns - * DS206: Consider reworking classes to avoid initClass - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const sinon = require('sinon') const modulePath = '../../../../app/js/DocumentManager.js' const SandboxedModule = require('sandboxed-module') @@ -20,8 +6,12 @@ const tk = require('timekeeper') describe('DocumentManager', function () { beforeEach(function () { - let Timer tk.freeze(new Date()) + this.Metrics = { + Timer: class Timer {}, + } + this.Metrics.Timer.prototype.done = sinon.stub() + this.DocumentManager = SandboxedModule.require(modulePath, { requires: { './RedisManager': (this.RedisManager = {}), @@ -31,17 +21,7 @@ describe('DocumentManager', function () { flushDocChangesAsync: sinon.stub(), flushProjectChangesAsync: sinon.stub(), }), - './Metrics': (this.Metrics = { - Timer: (Timer = (function () { - Timer = class Timer { - static initClass() { - this.prototype.done = sinon.stub() - } - } - Timer.initClass() - return Timer - })()), - }), + './Metrics': this.Metrics, './RealTimeRedisManager': (this.RealTimeRedisManager = {}), './DiffCodec': (this.DiffCodec = {}), './UpdateManager': (this.UpdateManager = {}), @@ -61,11 +41,11 @@ describe('DocumentManager', function () { this.pathname = '/a/b/c.tex' this.unflushedTime = Date.now() this.lastUpdatedAt = Date.now() - return (this.lastUpdatedBy = 'last-author-id') + this.lastUpdatedBy = 'last-author-id' }) afterEach(function () { - return tk.reset() + tk.reset() }) describe('flushAndDeleteDoc', function () { @@ -73,7 +53,7 @@ describe('DocumentManager', function () { beforeEach(function () { this.RedisManager.removeDocFromMemory = sinon.stub().callsArg(2) this.DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2) - return this.DocumentManager.flushAndDeleteDoc( + this.DocumentManager.flushAndDeleteDoc( this.project_id, this.doc_id, {}, @@ -82,67 +62,65 @@ describe('DocumentManager', function () { }) it('should flush the doc', function () { - return this.DocumentManager.flushDocIfLoaded + this.DocumentManager.flushDocIfLoaded .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should remove the doc from redis', function () { - return this.RedisManager.removeDocFromMemory + this.RedisManager.removeDocFromMemory .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should call the callback without error', function () { - return this.callback.calledWith(null).should.equal(true) + this.callback.calledWith(null).should.equal(true) }) it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + this.Metrics.Timer.prototype.done.called.should.equal(true) }) - return it('should flush to the history api', function () { - return this.HistoryManager.flushDocChangesAsync + it('should flush to the history api', function () { + this.HistoryManager.flushDocChangesAsync .calledWithExactly(this.project_id, this.doc_id) .should.equal(true) }) }) - return describe('when a flush error occurs', function () { + describe('when a flush error occurs', function () { beforeEach(function () { this.DocumentManager.flushDocIfLoaded = sinon .stub() .callsArgWith(2, new Error('boom!')) - return (this.RedisManager.removeDocFromMemory = sinon - .stub() - .callsArg(2)) + this.RedisManager.removeDocFromMemory = sinon.stub().callsArg(2) }) it('should not remove the doc from redis', function (done) { - return this.DocumentManager.flushAndDeleteDoc( + this.DocumentManager.flushAndDeleteDoc( this.project_id, this.doc_id, {}, error => { error.should.exist this.RedisManager.removeDocFromMemory.called.should.equal(false) - return done() + done() } ) }) - return describe('when ignoring flush errors', function () { - return it('should remove the doc from redis', function (done) { - return this.DocumentManager.flushAndDeleteDoc( + describe('when ignoring flush errors', function () { + it('should remove the doc from redis', function (done) { + this.DocumentManager.flushAndDeleteDoc( this.project_id, this.doc_id, { ignoreFlushErrors: true }, error => { - if (error != null) { + if (error) { return done(error) } this.RedisManager.removeDocFromMemory.called.should.equal(true) - return done() + done() } ) }) @@ -171,7 +149,7 @@ describe('DocumentManager', function () { .stub() .callsArgWith(1, null) this.PersistenceManager.setDoc = sinon.stub().yields() - return this.DocumentManager.flushDocIfLoaded( + this.DocumentManager.flushDocIfLoaded( this.project_id, this.doc_id, this.callback @@ -179,13 +157,13 @@ describe('DocumentManager', function () { }) it('should get the doc from redis', function () { - return this.RedisManager.getDoc + this.RedisManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should write the doc lines to the persistence layer', function () { - return this.PersistenceManager.setDoc + this.PersistenceManager.setDoc .calledWith( this.project_id, this.doc_id, @@ -199,21 +177,21 @@ describe('DocumentManager', function () { }) it('should call the callback without error', function () { - return this.callback.calledWith(null).should.equal(true) + this.callback.calledWith(null).should.equal(true) }) - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) - return describe('when the document is not in Redis', function () { + describe('when the document is not in Redis', function () { beforeEach(function () { this.RedisManager.getDoc = sinon .stub() .callsArgWith(2, null, null, null, null) this.PersistenceManager.setDoc = sinon.stub().yields() - return this.DocumentManager.flushDocIfLoaded( + this.DocumentManager.flushDocIfLoaded( this.project_id, this.doc_id, this.callback @@ -221,7 +199,7 @@ describe('DocumentManager', function () { }) it('should get the doc from redis', function () { - return this.RedisManager.getDoc + this.RedisManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) @@ -231,11 +209,11 @@ describe('DocumentManager', function () { }) it('should call the callback without error', function () { - return this.callback.calledWith(null).should.equal(true) + this.callback.calledWith(null).should.equal(true) }) - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) }) @@ -257,7 +235,7 @@ describe('DocumentManager', function () { this.RedisManager.getPreviousDocOps = sinon .stub() .callsArgWith(3, null, this.ops) - return this.DocumentManager.getDocAndRecentOps( + this.DocumentManager.getDocAndRecentOps( this.project_id, this.doc_id, this.fromVersion, @@ -266,19 +244,19 @@ describe('DocumentManager', function () { }) it('should get the doc', function () { - return this.DocumentManager.getDoc + this.DocumentManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should get the doc ops', function () { - return this.RedisManager.getPreviousDocOps + this.RedisManager.getPreviousDocOps .calledWith(this.doc_id, this.fromVersion, this.version) .should.equal(true) }) it('should call the callback with the doc info', function () { - return this.callback + this.callback .calledWith( null, this.lines, @@ -291,12 +269,12 @@ describe('DocumentManager', function () { .should.equal(true) }) - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) - return describe('with no previous version specified', function () { + describe('with no previous version specified', function () { beforeEach(function () { this.DocumentManager.getDoc = sinon .stub() @@ -312,7 +290,7 @@ describe('DocumentManager', function () { this.RedisManager.getPreviousDocOps = sinon .stub() .callsArgWith(3, null, this.ops) - return this.DocumentManager.getDocAndRecentOps( + this.DocumentManager.getDocAndRecentOps( this.project_id, this.doc_id, -1, @@ -321,17 +299,17 @@ describe('DocumentManager', function () { }) it('should get the doc', function () { - return this.DocumentManager.getDoc + this.DocumentManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should not need to get the doc ops', function () { - return this.RedisManager.getPreviousDocOps.called.should.equal(false) + this.RedisManager.getPreviousDocOps.called.should.equal(false) }) it('should call the callback with the doc info', function () { - return this.callback + this.callback .calledWith( null, this.lines, @@ -344,8 +322,8 @@ describe('DocumentManager', function () { .should.equal(true) }) - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) }) @@ -365,21 +343,17 @@ describe('DocumentManager', function () { this.projectHistoryId, this.unflushedTime ) - return this.DocumentManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) + this.DocumentManager.getDoc(this.project_id, this.doc_id, this.callback) }) it('should get the doc from Redis', function () { - return this.RedisManager.getDoc + this.RedisManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should call the callback with the doc info', function () { - return this.callback + this.callback .calledWith( null, this.lines, @@ -393,12 +367,12 @@ describe('DocumentManager', function () { .should.equal(true) }) - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) - return describe('when the doc does not exist in Redis', function () { + describe('when the doc does not exist in Redis', function () { beforeEach(function () { this.RedisManager.getDoc = sinon .stub() @@ -417,27 +391,23 @@ describe('DocumentManager', function () { ) this.RedisManager.putDocInMemory = sinon.stub().yields() this.RedisManager.setHistoryType = sinon.stub().yields() - return this.DocumentManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) + this.DocumentManager.getDoc(this.project_id, this.doc_id, this.callback) }) it('should try to get the doc from Redis', function () { - return this.RedisManager.getDoc + this.RedisManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should get the doc from the PersistenceManager', function () { - return this.PersistenceManager.getDoc + this.PersistenceManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should set the doc in Redis', function () { - return this.RedisManager.putDocInMemory + this.RedisManager.putDocInMemory .calledWith( this.project_id, this.doc_id, @@ -451,13 +421,13 @@ describe('DocumentManager', function () { }) it('should set the history type in Redis', function () { - return this.RedisManager.setHistoryType + this.RedisManager.setHistoryType .calledWith(this.doc_id, this.projectHistoryType) .should.equal(true) }) it('should call the callback with the doc info', function () { - return this.callback + this.callback .calledWith( null, this.lines, @@ -471,14 +441,14 @@ describe('DocumentManager', function () { .should.equal(true) }) - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) }) describe('setDoc', function () { - return describe('with plain tex lines', function () { + describe('with plain tex lines', function () { beforeEach(function () { this.beforeLines = ['before', 'lines'] this.afterLines = ['after', 'lines'] @@ -504,14 +474,12 @@ describe('DocumentManager', function () { .callsArgWith(2, null, this.ops) this.UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null) this.DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) - return (this.DocumentManager.flushAndDeleteDoc = sinon - .stub() - .callsArg(3)) + this.DocumentManager.flushAndDeleteDoc = sinon.stub().callsArg(3) }) describe('when already loaded', function () { beforeEach(function () { - return this.DocumentManager.setDoc( + this.DocumentManager.setDoc( this.project_id, this.doc_id, this.afterLines, @@ -523,19 +491,19 @@ describe('DocumentManager', function () { }) it('should get the current doc lines', function () { - return this.DocumentManager.getDoc + this.DocumentManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should return a diff of the old and new lines', function () { - return this.DiffCodec.diffAsShareJsOp + this.DiffCodec.diffAsShareJsOp .calledWith(this.beforeLines, this.afterLines) .should.equal(true) }) it('should apply the diff as a ShareJS op', function () { - return this.UpdateManager.applyUpdate + this.UpdateManager.applyUpdate .calledWith(this.project_id, this.doc_id, { doc: this.doc_id, v: this.version, @@ -550,23 +518,23 @@ describe('DocumentManager', function () { }) it('should flush the doc to Mongo', function () { - return this.DocumentManager.flushDocIfLoaded + this.DocumentManager.flushDocIfLoaded .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should not flush the project history', function () { - return this.HistoryManager.flushProjectChangesAsync.called.should.equal( + this.HistoryManager.flushProjectChangesAsync.called.should.equal( false ) }) it('should call the callback', function () { - return this.callback.calledWith(null).should.equal(true) + this.callback.calledWith(null).should.equal(true) }) - return it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + it('should time the execution', function () { + this.Metrics.Timer.prototype.done.called.should.equal(true) }) }) @@ -583,7 +551,7 @@ describe('DocumentManager', function () { null, false ) - return this.DocumentManager.setDoc( + this.DocumentManager.setDoc( this.project_id, this.doc_id, this.afterLines, @@ -595,13 +563,13 @@ describe('DocumentManager', function () { }) it('should flush and delete the doc from the doc updater', function () { - return this.DocumentManager.flushAndDeleteDoc + this.DocumentManager.flushAndDeleteDoc .calledWith(this.project_id, this.doc_id, {}) .should.equal(true) }) - return it('should not flush the project history', function () { - return this.HistoryManager.flushProjectChangesAsync + it('should not flush the project history', function () { + this.HistoryManager.flushProjectChangesAsync .calledWithExactly(this.project_id) .should.equal(true) }) @@ -609,7 +577,7 @@ describe('DocumentManager', function () { describe('without new lines', function () { beforeEach(function () { - return this.DocumentManager.setDoc( + this.DocumentManager.setDoc( this.project_id, this.doc_id, null, @@ -621,17 +589,15 @@ describe('DocumentManager', function () { }) it('should return the callback with an error', function () { - return this.callback.calledWith( - new Error('No lines were passed to setDoc') - ) + this.callback.calledWith(new Error('No lines were passed to setDoc')) }) - return it('should not try to get the doc lines', function () { - return this.DocumentManager.getDoc.called.should.equal(false) + it('should not try to get the doc lines', function () { + this.DocumentManager.getDoc.called.should.equal(false) }) }) - return describe('with the undoing flag', function () { + describe('with the undoing flag', function () { beforeEach(function () { // Copy ops so we don't interfere with other tests this.ops = [ @@ -641,7 +607,7 @@ describe('DocumentManager', function () { this.DiffCodec.diffAsShareJsOp = sinon .stub() .callsArgWith(2, null, this.ops) - return this.DocumentManager.setDoc( + this.DocumentManager.setDoc( this.project_id, this.doc_id, this.afterLines, @@ -652,8 +618,8 @@ describe('DocumentManager', function () { ) }) - return it('should set the undo flag on each op', function () { - return Array.from(this.ops).map(op => op.u.should.equal(true)) + it('should set the undo flag on each op', function () { + this.ops.map(op => op.u.should.equal(true)) }) }) }) @@ -678,12 +644,12 @@ describe('DocumentManager', function () { this.RangesManager.acceptChanges = sinon .stub() .yields(null, this.updated_ranges) - return (this.RedisManager.updateDocument = sinon.stub().yields()) + this.RedisManager.updateDocument = sinon.stub().yields() }) describe('successfully with a single change', function () { beforeEach(function () { - return this.DocumentManager.acceptChanges( + this.DocumentManager.acceptChanges( this.project_id, this.doc_id, [this.change_id], @@ -692,19 +658,19 @@ describe('DocumentManager', function () { }) it("should get the document's current ranges", function () { - return this.DocumentManager.getDoc + this.DocumentManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should apply the accept change to the ranges', function () { - return this.RangesManager.acceptChanges + this.RangesManager.acceptChanges .calledWith([this.change_id], this.ranges) .should.equal(true) }) it('should save the updated ranges', function () { - return this.RedisManager.updateDocument + this.RedisManager.updateDocument .calledWith( this.project_id, this.doc_id, @@ -717,14 +683,14 @@ describe('DocumentManager', function () { .should.equal(true) }) - return it('should call the callback', function () { - return this.callback.called.should.equal(true) + it('should call the callback', function () { + this.callback.called.should.equal(true) }) }) describe('successfully with multiple changes', function () { beforeEach(function () { - return this.DocumentManager.acceptChanges( + this.DocumentManager.acceptChanges( this.project_id, this.doc_id, this.change_ids, @@ -732,19 +698,19 @@ describe('DocumentManager', function () { ) }) - return it('should apply the accept change to the ranges', function () { - return this.RangesManager.acceptChanges + it('should apply the accept change to the ranges', function () { + this.RangesManager.acceptChanges .calledWith(this.change_ids, this.ranges) .should.equal(true) }) }) - return describe('when the doc is not found', function () { + describe('when the doc is not found', function () { beforeEach(function () { this.DocumentManager.getDoc = sinon .stub() .yields(null, null, null, null) - return this.DocumentManager.acceptChanges( + this.DocumentManager.acceptChanges( this.project_id, this.doc_id, [this.change_id], @@ -753,11 +719,11 @@ describe('DocumentManager', function () { }) it('should not save anything', function () { - return this.RedisManager.updateDocument.called.should.equal(false) + this.RedisManager.updateDocument.called.should.equal(false) }) - return it('should call the callback with a not found error', function () { - return this.callback + it('should call the callback with a not found error', function () { + this.callback .calledWith(sinon.match.instanceOf(Errors.NotFoundError)) .should.equal(true) }) @@ -777,12 +743,12 @@ describe('DocumentManager', function () { this.RangesManager.deleteComment = sinon .stub() .yields(null, this.updated_ranges) - return (this.RedisManager.updateDocument = sinon.stub().yields()) + this.RedisManager.updateDocument = sinon.stub().yields() }) describe('successfully', function () { beforeEach(function () { - return this.DocumentManager.deleteComment( + this.DocumentManager.deleteComment( this.project_id, this.doc_id, this.comment_id, @@ -791,19 +757,19 @@ describe('DocumentManager', function () { }) it("should get the document's current ranges", function () { - return this.DocumentManager.getDoc + this.DocumentManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should delete the comment from the ranges', function () { - return this.RangesManager.deleteComment + this.RangesManager.deleteComment .calledWith(this.comment_id, this.ranges) .should.equal(true) }) it('should save the updated ranges', function () { - return this.RedisManager.updateDocument + this.RedisManager.updateDocument .calledWith( this.project_id, this.doc_id, @@ -816,17 +782,17 @@ describe('DocumentManager', function () { .should.equal(true) }) - return it('should call the callback', function () { - return this.callback.called.should.equal(true) + it('should call the callback', function () { + this.callback.called.should.equal(true) }) }) - return describe('when the doc is not found', function () { + describe('when the doc is not found', function () { beforeEach(function () { this.DocumentManager.getDoc = sinon .stub() .yields(null, null, null, null) - return this.DocumentManager.acceptChanges( + this.DocumentManager.acceptChanges( this.project_id, this.doc_id, [this.comment_id], @@ -835,11 +801,11 @@ describe('DocumentManager', function () { }) it('should not save anything', function () { - return this.RedisManager.updateDocument.called.should.equal(false) + this.RedisManager.updateDocument.called.should.equal(false) }) - return it('should call the callback with a not found error', function () { - return this.callback + it('should call the callback with a not found error', function () { + this.callback .calledWith(sinon.match.instanceOf(Errors.NotFoundError)) .should.equal(true) }) @@ -848,7 +814,7 @@ describe('DocumentManager', function () { describe('getDocAndFlushIfOld', function () { beforeEach(function () { - return (this.DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2)) + this.DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2) }) describe('when the doc is in Redis', function () { @@ -867,7 +833,7 @@ describe('DocumentManager', function () { Date.now() - 1e9, true ) - return this.DocumentManager.getDocAndFlushIfOld( + this.DocumentManager.getDocAndFlushIfOld( this.project_id, this.doc_id, this.callback @@ -875,25 +841,25 @@ describe('DocumentManager', function () { }) it('should get the doc', function () { - return this.DocumentManager.getDoc + this.DocumentManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should flush the doc', function () { - return this.DocumentManager.flushDocIfLoaded + this.DocumentManager.flushDocIfLoaded .calledWith(this.project_id, this.doc_id) .should.equal(true) }) - return it('should call the callback with the lines and versions', function () { - return this.callback + it('should call the callback with the lines and versions', function () { + this.callback .calledWith(null, this.lines, this.version) .should.equal(true) }) }) - return describe("and has only changes that don't need to be flushed", function () { + describe("and has only changes that don't need to be flushed", function () { beforeEach(function () { this.DocumentManager.getDoc = sinon .stub() @@ -907,7 +873,7 @@ describe('DocumentManager', function () { Date.now() - 100, true ) - return this.DocumentManager.getDocAndFlushIfOld( + this.DocumentManager.getDocAndFlushIfOld( this.project_id, this.doc_id, this.callback @@ -915,26 +881,24 @@ describe('DocumentManager', function () { }) it('should get the doc', function () { - return this.DocumentManager.getDoc + this.DocumentManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should not flush the doc', function () { - return this.DocumentManager.flushDocIfLoaded.called.should.equal( - false - ) + this.DocumentManager.flushDocIfLoaded.called.should.equal(false) }) - return it('should call the callback with the lines and versions', function () { - return this.callback + it('should call the callback with the lines and versions', function () { + this.callback .calledWith(null, this.lines, this.version) .should.equal(true) }) }) }) - return describe('when the doc is not in Redis', function () { + describe('when the doc is not in Redis', function () { beforeEach(function () { this.DocumentManager.getDoc = sinon .stub() @@ -947,7 +911,7 @@ describe('DocumentManager', function () { null, false ) - return this.DocumentManager.getDocAndFlushIfOld( + this.DocumentManager.getDocAndFlushIfOld( this.project_id, this.doc_id, this.callback @@ -955,17 +919,17 @@ describe('DocumentManager', function () { }) it('should get the doc', function () { - return this.DocumentManager.getDoc + this.DocumentManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should not flush the doc', function () { - return this.DocumentManager.flushDocIfLoaded.called.should.equal(false) + this.DocumentManager.flushDocIfLoaded.called.should.equal(false) }) - return it('should call the callback with the lines and versions', function () { - return this.callback + it('should call the callback with the lines and versions', function () { + this.callback .calledWith(null, this.lines, this.version) .should.equal(true) }) @@ -975,12 +939,12 @@ describe('DocumentManager', function () { describe('renameDoc', function () { beforeEach(function () { this.update = 'some-update' - return (this.RedisManager.renameDoc = sinon.stub().yields()) + this.RedisManager.renameDoc = sinon.stub().yields() }) - return describe('successfully', function () { + describe('successfully', function () { beforeEach(function () { - return this.DocumentManager.renameDoc( + this.DocumentManager.renameDoc( this.project_id, this.doc_id, this.user_id, @@ -991,7 +955,7 @@ describe('DocumentManager', function () { }) it('should rename the document', function () { - return this.RedisManager.renameDoc + this.RedisManager.renameDoc .calledWith( this.project_id, this.doc_id, @@ -1002,13 +966,13 @@ describe('DocumentManager', function () { .should.equal(true) }) - return it('should call the callback', function () { - return this.callback.called.should.equal(true) + it('should call the callback', function () { + this.callback.called.should.equal(true) }) }) }) - return describe('resyncDocContents', function () { + describe('resyncDocContents', function () { describe('when doc is loaded in redis', function () { beforeEach(function () { this.RedisManager.getDoc = sinon @@ -1023,7 +987,7 @@ describe('DocumentManager', function () { this.projectHistoryId ) this.ProjectHistoryRedisManager.queueResyncDocContent = sinon.stub() - return this.DocumentManager.resyncDocContents( + this.DocumentManager.resyncDocContents( this.project_id, this.doc_id, this.callback @@ -1031,13 +995,13 @@ describe('DocumentManager', function () { }) it('gets the doc contents from redis', function () { - return this.RedisManager.getDoc + this.RedisManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) - return it('queues a resync doc content update', function () { - return this.ProjectHistoryRedisManager.queueResyncDocContent + it('queues a resync doc content update', function () { + this.ProjectHistoryRedisManager.queueResyncDocContent .calledWith( this.project_id, this.projectHistoryId, @@ -1051,13 +1015,12 @@ describe('DocumentManager', function () { }) }) - return describe('when doc is not loaded in redis', function () { + describe('when doc is not loaded in redis', function () { beforeEach(function () { this.RedisManager.getDoc = sinon.stub().callsArgWith(2, null) this.PersistenceManager.getDoc = sinon .stub() - .callsArgWith( - 2, + .yields( null, this.lines, this.version, @@ -1066,7 +1029,7 @@ describe('DocumentManager', function () { this.projectHistoryId ) this.ProjectHistoryRedisManager.queueResyncDocContent = sinon.stub() - return this.DocumentManager.resyncDocContents( + this.DocumentManager.resyncDocContents( this.project_id, this.doc_id, this.callback @@ -1074,19 +1037,19 @@ describe('DocumentManager', function () { }) it('tries to get the doc contents from redis', function () { - return this.RedisManager.getDoc + this.RedisManager.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('gets the doc contents from web', function () { - return this.PersistenceManager.getDoc - .calledWith(this.project_id, this.doc_id) + this.PersistenceManager.getDoc + .calledWith(this.project_id, this.doc_id, { peek: true }) .should.equal(true) }) - return it('queues a resync doc content update', function () { - return this.ProjectHistoryRedisManager.queueResyncDocContent + it('queues a resync doc content update', function () { + this.ProjectHistoryRedisManager.queueResyncDocContent .calledWith( this.project_id, this.projectHistoryId, diff --git a/services/document-updater/test/unit/js/PersistenceManager/PersistenceManagerTests.js b/services/document-updater/test/unit/js/PersistenceManager/PersistenceManagerTests.js index cc669481f7..aa25a22345 100644 --- a/services/document-updater/test/unit/js/PersistenceManager/PersistenceManagerTests.js +++ b/services/document-updater/test/unit/js/PersistenceManager/PersistenceManagerTests.js @@ -1,15 +1,3 @@ -/* eslint-disable - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS206: Consider reworking classes to avoid initClass - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const sinon = require('sinon') const modulePath = '../../../../app/js/PersistenceManager.js' const SandboxedModule = require('sandboxed-module') @@ -17,25 +5,20 @@ const Errors = require('../../../../app/js/Errors') describe('PersistenceManager', function () { beforeEach(function () { - let Timer this.request = sinon.stub() this.request.defaults = () => this.request + this.Metrics = { + Timer: class Timer {}, + inc: sinon.stub(), + } + this.Metrics.Timer.prototype.done = sinon.stub() + this.Settings = {} + this.PersistenceManager = SandboxedModule.require(modulePath, { requires: { requestretry: this.request, - '@overleaf/settings': (this.Settings = {}), - './Metrics': (this.Metrics = { - Timer: (Timer = (function () { - Timer = class Timer { - static initClass() { - this.prototype.done = sinon.stub() - } - } - Timer.initClass() - return Timer - })()), - inc: sinon.stub(), - }), + '@overleaf/settings': this.Settings, + './Metrics': this.Metrics, './Errors': Errors, }, }) @@ -49,24 +32,24 @@ describe('PersistenceManager', function () { this.pathname = '/a/b/c.tex' this.lastUpdatedAt = Date.now() this.lastUpdatedBy = 'last-author-id' - return (this.Settings.apis = { + this.Settings.apis = { web: { url: (this.url = 'www.example.com'), user: (this.user = 'sharelatex'), pass: (this.pass = 'password'), }, - }) + } }) describe('getDoc', function () { beforeEach(function () { - return (this.webResponse = { + this.webResponse = { lines: this.lines, version: this.version, ranges: this.ranges, pathname: this.pathname, projectHistoryId: this.projectHistoryId, - }) + } }) describe('with a successful response from the web api', function () { @@ -77,7 +60,7 @@ describe('PersistenceManager', function () { { statusCode: 200 }, JSON.stringify(this.webResponse) ) - return this.PersistenceManager.getDoc( + this.PersistenceManager.getDoc( this.project_id, this.doc_id, this.callback @@ -85,7 +68,7 @@ describe('PersistenceManager', function () { }) it('should call the web api', function () { - return this.request + this.request .calledWith({ url: `${this.url}/project/${this.project_id}/doc/${this.doc_id}`, method: 'GET', @@ -104,7 +87,7 @@ describe('PersistenceManager', function () { }) it('should call the callback with the doc lines, version and ranges', function () { - return this.callback + this.callback .calledWith( null, this.lines, @@ -117,22 +100,58 @@ describe('PersistenceManager', function () { }) it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + this.Metrics.Timer.prototype.done.called.should.equal(true) }) - return it('should increment the metric', function () { - return this.Metrics.inc + it('should increment the metric', function () { + this.Metrics.inc .calledWith('getDoc', 1, { status: 200 }) .should.equal(true) }) }) + describe('with the peek option', function () { + beforeEach(function () { + this.request.yields( + null, + { statusCode: 200 }, + JSON.stringify(this.webResponse) + ) + this.PersistenceManager.getDoc( + this.project_id, + this.doc_id, + { peek: true }, + this.callback + ) + }) + + it('should call the web api with a peek param', function () { + this.request + .calledWith({ + url: `${this.url}/project/${this.project_id}/doc/${this.doc_id}`, + qs: { peek: 'true' }, + method: 'GET', + headers: { + accept: 'application/json', + }, + auth: { + user: this.user, + pass: this.pass, + sendImmediately: true, + }, + jar: false, + timeout: 5000, + }) + .should.equal(true) + }) + }) + describe('when request returns an error', function () { beforeEach(function () { this.error = new Error('oops') this.error.code = 'EOOPS' this.request.callsArgWith(1, this.error, null, null) - return this.PersistenceManager.getDoc( + this.PersistenceManager.getDoc( this.project_id, this.doc_id, this.callback @@ -140,7 +159,7 @@ describe('PersistenceManager', function () { }) it('should return a generic connection error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Error) @@ -150,11 +169,11 @@ describe('PersistenceManager', function () { }) it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + this.Metrics.Timer.prototype.done.called.should.equal(true) }) - return it('should increment the metric', function () { - return this.Metrics.inc + it('should increment the metric', function () { + this.Metrics.inc .calledWith('getDoc', 1, { status: 'EOOPS' }) .should.equal(true) }) @@ -163,7 +182,7 @@ describe('PersistenceManager', function () { describe('when the request returns 404', function () { beforeEach(function () { this.request.callsArgWith(1, null, { statusCode: 404 }, '') - return this.PersistenceManager.getDoc( + this.PersistenceManager.getDoc( this.project_id, this.doc_id, this.callback @@ -171,17 +190,17 @@ describe('PersistenceManager', function () { }) it('should return a NotFoundError', function () { - return this.callback + this.callback .calledWith(sinon.match.instanceOf(Errors.NotFoundError)) .should.equal(true) }) it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + this.Metrics.Timer.prototype.done.called.should.equal(true) }) - return it('should increment the metric', function () { - return this.Metrics.inc + it('should increment the metric', function () { + this.Metrics.inc .calledWith('getDoc', 1, { status: 404 }) .should.equal(true) }) @@ -190,7 +209,7 @@ describe('PersistenceManager', function () { describe('when the request returns an error status code', function () { beforeEach(function () { this.request.callsArgWith(1, null, { statusCode: 500 }, '') - return this.PersistenceManager.getDoc( + this.PersistenceManager.getDoc( this.project_id, this.doc_id, this.callback @@ -198,17 +217,17 @@ describe('PersistenceManager', function () { }) it('should return an error', function () { - return this.callback + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + this.Metrics.Timer.prototype.done.called.should.equal(true) }) - return it('should increment the metric', function () { - return this.Metrics.inc + it('should increment the metric', function () { + this.Metrics.inc .calledWith('getDoc', 1, { status: 500 }) .should.equal(true) }) @@ -223,15 +242,15 @@ describe('PersistenceManager', function () { { statusCode: 200 }, JSON.stringify(this.webResponse) ) - return this.PersistenceManager.getDoc( + this.PersistenceManager.getDoc( this.project_id, this.doc_id, this.callback ) }) - return it('should return and error', function () { - return this.callback + it('should return and error', function () { + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) @@ -246,21 +265,21 @@ describe('PersistenceManager', function () { { statusCode: 200 }, JSON.stringify(this.webResponse) ) - return this.PersistenceManager.getDoc( + this.PersistenceManager.getDoc( this.project_id, this.doc_id, this.callback ) }) - return it('should return and error', function () { - return this.callback + it('should return and error', function () { + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) }) - return describe('when request returns an doc without a pathname', function () { + describe('when request returns an doc without a pathname', function () { beforeEach(function () { delete this.webResponse.pathname this.request.callsArgWith( @@ -269,26 +288,26 @@ describe('PersistenceManager', function () { { statusCode: 200 }, JSON.stringify(this.webResponse) ) - return this.PersistenceManager.getDoc( + this.PersistenceManager.getDoc( this.project_id, this.doc_id, this.callback ) }) - return it('should return and error', function () { - return this.callback + it('should return and error', function () { + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) }) }) - return describe('setDoc', function () { + describe('setDoc', function () { describe('with a successful response from the web api', function () { beforeEach(function () { this.request.callsArgWith(1, null, { statusCode: 200 }) - return this.PersistenceManager.setDoc( + this.PersistenceManager.setDoc( this.project_id, this.doc_id, this.lines, @@ -301,7 +320,7 @@ describe('PersistenceManager', function () { }) it('should call the web api', function () { - return this.request + this.request .calledWith({ url: `${this.url}/project/${this.project_id}/doc/${this.doc_id}`, json: { @@ -324,15 +343,15 @@ describe('PersistenceManager', function () { }) it('should call the callback without error', function () { - return this.callback.calledWith(null).should.equal(true) + this.callback.calledWith(null).should.equal(true) }) it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + this.Metrics.Timer.prototype.done.called.should.equal(true) }) - return it('should increment the metric', function () { - return this.Metrics.inc + it('should increment the metric', function () { + this.Metrics.inc .calledWith('setDoc', 1, { status: 200 }) .should.equal(true) }) @@ -343,7 +362,7 @@ describe('PersistenceManager', function () { this.error = new Error('oops') this.error.code = 'EOOPS' this.request.callsArgWith(1, this.error, null, null) - return this.PersistenceManager.setDoc( + this.PersistenceManager.setDoc( this.project_id, this.doc_id, this.lines, @@ -356,7 +375,7 @@ describe('PersistenceManager', function () { }) it('should return a generic connection error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Error) @@ -366,11 +385,11 @@ describe('PersistenceManager', function () { }) it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + this.Metrics.Timer.prototype.done.called.should.equal(true) }) - return it('should increment the metric', function () { - return this.Metrics.inc + it('should increment the metric', function () { + this.Metrics.inc .calledWith('setDoc', 1, { status: 'EOOPS' }) .should.equal(true) }) @@ -379,7 +398,7 @@ describe('PersistenceManager', function () { describe('when the request returns 404', function () { beforeEach(function () { this.request.callsArgWith(1, null, { statusCode: 404 }, '') - return this.PersistenceManager.setDoc( + this.PersistenceManager.setDoc( this.project_id, this.doc_id, this.lines, @@ -392,26 +411,26 @@ describe('PersistenceManager', function () { }) it('should return a NotFoundError', function () { - return this.callback + this.callback .calledWith(sinon.match.instanceOf(Errors.NotFoundError)) .should.equal(true) }) it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + this.Metrics.Timer.prototype.done.called.should.equal(true) }) - return it('should increment the metric', function () { - return this.Metrics.inc + it('should increment the metric', function () { + this.Metrics.inc .calledWith('setDoc', 1, { status: 404 }) .should.equal(true) }) }) - return describe('when the request returns an error status code', function () { + describe('when the request returns an error status code', function () { beforeEach(function () { this.request.callsArgWith(1, null, { statusCode: 500 }, '') - return this.PersistenceManager.setDoc( + this.PersistenceManager.setDoc( this.project_id, this.doc_id, this.lines, @@ -424,17 +443,17 @@ describe('PersistenceManager', function () { }) it('should return an error', function () { - return this.callback + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) it('should time the execution', function () { - return this.Metrics.Timer.prototype.done.called.should.equal(true) + this.Metrics.Timer.prototype.done.called.should.equal(true) }) - return it('should increment the metric', function () { - return this.Metrics.inc + it('should increment the metric', function () { + this.Metrics.inc .calledWith('setDoc', 1, { status: 500 }) .should.equal(true) }) diff --git a/services/web/app/src/Features/Docstore/DocstoreManager.js b/services/web/app/src/Features/Docstore/DocstoreManager.js index f7327a5f3a..87995f342e 100644 --- a/services/web/app/src/Features/Docstore/DocstoreManager.js +++ b/services/web/app/src/Features/Docstore/DocstoreManager.js @@ -1,303 +1,277 @@ -/* eslint-disable - camelcase, - node/handle-callback-err, - max-len, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ +const { promisify } = require('util') +const { promisifyMultiResult } = require('../../util/promises') const request = require('request').defaults({ jar: false }) const OError = require('@overleaf/o-error') const logger = require('logger-sharelatex') const settings = require('@overleaf/settings') const Errors = require('../Errors/Errors') -const { promisifyAll } = require('../../util/promises') const TIMEOUT = 30 * 1000 // request timeout -const DocstoreManager = { - deleteDoc(project_id, doc_id, name, deletedAt, callback) { - if (callback == null) { - callback = function (error) {} +function deleteDoc(projectId, docId, name, deletedAt, callback) { + const url = `${settings.apis.docstore.url}/project/${projectId}/doc/${docId}` + const docMetaData = { deleted: true, deletedAt, name } + const options = { url, json: docMetaData, timeout: TIMEOUT } + request.patch(options, (error, res) => { + if (error) { + return callback(error) } - const url = `${settings.apis.docstore.url}/project/${project_id}/doc/${doc_id}` - const docMetaData = { deleted: true, deletedAt, name } - const options = { url, json: docMetaData, timeout: TIMEOUT } - request.patch(options, function (error, res) { - if (error != null) { + if (res.statusCode >= 200 && res.statusCode < 300) { + callback(null) + } else if (res.statusCode === 404) { + error = new Errors.NotFoundError({ + message: 'tried to delete doc not in docstore', + info: { + projectId, + docId, + }, + }) + callback(error) // maybe suppress the error when delete doc which is not present? + } else { + error = new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { + projectId, + docId, + } + ) + callback(error) + } + }) +} + +function getAllDocs(projectId, callback) { + const url = `${settings.apis.docstore.url}/project/${projectId}/doc` + request.get( + { + url, + timeout: TIMEOUT, + json: true, + }, + (error, res, docs) => { + if (error) { return callback(error) } if (res.statusCode >= 200 && res.statusCode < 300) { - return callback(null) - } else if (res.statusCode === 404) { - error = new Errors.NotFoundError({ - message: 'tried to delete doc not in docstore', - info: { - project_id, - doc_id, - }, - }) - return callback(error) // maybe suppress the error when delete doc which is not present? + callback(null, docs) } else { error = new OError( `docstore api responded with non-success code: ${res.statusCode}`, - { - project_id, - doc_id, - } - ) - return callback(error) - } - }) - }, - - getAllDocs(project_id, callback) { - if (callback == null) { - callback = function (error) {} - } - const url = `${settings.apis.docstore.url}/project/${project_id}/doc` - return request.get( - { - url, - timeout: TIMEOUT, - json: true, - }, - function (error, res, docs) { - if (error != null) { - return callback(error) - } - if (res.statusCode >= 200 && res.statusCode < 300) { - return callback(null, docs) - } else { - error = new OError( - `docstore api responded with non-success code: ${res.statusCode}`, - { project_id } - ) - return callback(error) - } - } - ) - }, - - getAllDeletedDocs(project_id, callback) { - const url = `${settings.apis.docstore.url}/project/${project_id}/doc-deleted` - request.get( - { url, timeout: TIMEOUT, json: true }, - function (error, res, docs) { - if (error) { - callback( - OError.tag(error, 'could not get deleted docs from docstore') - ) - } else if (res.statusCode === 200) { - callback(null, docs) - } else { - callback( - new OError( - `docstore api responded with non-success code: ${res.statusCode}`, - { project_id } - ) - ) - } - } - ) - }, - - getAllRanges(project_id, callback) { - if (callback == null) { - callback = function (error) {} - } - const url = `${settings.apis.docstore.url}/project/${project_id}/ranges` - return request.get( - { - url, - timeout: TIMEOUT, - json: true, - }, - function (error, res, docs) { - if (error != null) { - return callback(error) - } - if (res.statusCode >= 200 && res.statusCode < 300) { - return callback(null, docs) - } else { - error = new OError( - `docstore api responded with non-success code: ${res.statusCode}`, - { project_id } - ) - return callback(error) - } - } - ) - }, - - getDoc(project_id, doc_id, options, callback) { - if (options == null) { - options = {} - } - if (callback == null) { - callback = function (error, lines, rev, version) {} - } - if (typeof options === 'function') { - callback = options - options = {} - } - let url = `${settings.apis.docstore.url}/project/${project_id}/doc/${doc_id}` - if (options.include_deleted) { - url += '?include_deleted=true' - } - return request.get( - { - url, - timeout: TIMEOUT, - json: true, - }, - function (error, res, doc) { - if (error != null) { - return callback(error) - } - if (res.statusCode >= 200 && res.statusCode < 300) { - logger.log( - { doc_id, project_id, version: doc.version, rev: doc.rev }, - 'got doc from docstore api' - ) - return callback(null, doc.lines, doc.rev, doc.version, doc.ranges) - } else if (res.statusCode === 404) { - error = new Errors.NotFoundError({ - message: 'doc not found in docstore', - info: { - project_id, - doc_id, - }, - }) - return callback(error) - } else { - error = new OError( - `docstore api responded with non-success code: ${res.statusCode}`, - { - project_id, - doc_id, - } - ) - return callback(error) - } - } - ) - }, - - isDocDeleted(project_id, doc_id, callback) { - const url = `${settings.apis.docstore.url}/project/${project_id}/doc/${doc_id}/deleted` - request.get( - { url, timeout: TIMEOUT, json: true }, - function (err, res, body) { - if (err) { - callback(err) - } else if (res.statusCode === 200) { - callback(null, body.deleted) - } else if (res.statusCode === 404) { - callback( - new Errors.NotFoundError({ - message: 'doc does not exist in project', - info: { project_id, doc_id }, - }) - ) - } else { - callback( - new OError( - `docstore api responded with non-success code: ${res.statusCode}`, - { project_id, doc_id } - ) - ) - } - } - ) - }, - - updateDoc(project_id, doc_id, lines, version, ranges, callback) { - if (callback == null) { - callback = function (error, modified, rev) {} - } - const url = `${settings.apis.docstore.url}/project/${project_id}/doc/${doc_id}` - return request.post( - { - url, - timeout: TIMEOUT, - json: { - lines, - version, - ranges, - }, - }, - function (error, res, result) { - if (error != null) { - return callback(error) - } - if (res.statusCode >= 200 && res.statusCode < 300) { - logger.log( - { project_id, doc_id }, - 'update doc in docstore url finished' - ) - return callback(null, result.modified, result.rev) - } else { - error = new OError( - `docstore api responded with non-success code: ${res.statusCode}`, - { project_id, doc_id } - ) - return callback(error) - } - } - ) - }, - - archiveProject(project_id, callback) { - DocstoreManager._operateOnProject(project_id, 'archive', callback) - }, - - unarchiveProject(project_id, callback) { - DocstoreManager._operateOnProject(project_id, 'unarchive', callback) - }, - - destroyProject(project_id, callback) { - DocstoreManager._operateOnProject(project_id, 'destroy', callback) - }, - - _operateOnProject(project_id, method, callback) { - const url = `${settings.apis.docstore.url}/project/${project_id}/${method}` - logger.log({ project_id }, `calling ${method} for project in docstore`) - // use default timeout for archiving/unarchiving/destroying - request.post(url, function (err, res, docs) { - if (err != null) { - OError.tag(err, `error calling ${method} project in docstore`, { - project_id, - }) - return callback(err) - } - - if (res.statusCode >= 200 && res.statusCode < 300) { - callback() - } else { - const error = new Error( - `docstore api responded with non-success code: ${res.statusCode}` - ) - logger.warn( - { err: error, project_id }, - `error calling ${method} project in docstore` + { projectId } ) callback(error) } - }) - }, + } + ) } -module.exports = DocstoreManager -module.exports.promises = promisifyAll(DocstoreManager, { - multiResult: { - getDoc: ['lines', 'rev', 'version', 'ranges'], - updateDoc: ['modified', 'rev'], +function getAllDeletedDocs(projectId, callback) { + const url = `${settings.apis.docstore.url}/project/${projectId}/doc-deleted` + request.get({ url, timeout: TIMEOUT, json: true }, (error, res, docs) => { + if (error) { + callback(OError.tag(error, 'could not get deleted docs from docstore')) + } else if (res.statusCode === 200) { + callback(null, docs) + } else { + callback( + new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { projectId } + ) + ) + } + }) +} + +function getAllRanges(projectId, callback) { + const url = `${settings.apis.docstore.url}/project/${projectId}/ranges` + request.get( + { + url, + timeout: TIMEOUT, + json: true, + }, + (error, res, docs) => { + if (error) { + return callback(error) + } + if (res.statusCode >= 200 && res.statusCode < 300) { + callback(null, docs) + } else { + error = new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { projectId } + ) + callback(error) + } + } + ) +} + +function getDoc(projectId, docId, options, callback) { + if (options == null) { + options = {} + } + if (typeof options === 'function') { + callback = options + options = {} + } + const requestParams = { timeout: TIMEOUT, json: true } + if (options.peek) { + requestParams.url = `${settings.apis.docstore.url}/project/${projectId}/doc/${docId}/peek` + } else { + requestParams.url = `${settings.apis.docstore.url}/project/${projectId}/doc/${docId}` + } + if (options.include_deleted) { + requestParams.qs = { include_deleted: 'true' } + } + request.get(requestParams, (error, res, doc) => { + if (error) { + return callback(error) + } + if (res.statusCode >= 200 && res.statusCode < 300) { + logger.log( + { docId, projectId, version: doc.version, rev: doc.rev }, + 'got doc from docstore api' + ) + callback(null, doc.lines, doc.rev, doc.version, doc.ranges) + } else if (res.statusCode === 404) { + error = new Errors.NotFoundError({ + message: 'doc not found in docstore', + info: { + projectId, + docId, + }, + }) + callback(error) + } else { + error = new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { + projectId, + docId, + } + ) + callback(error) + } + }) +} + +function isDocDeleted(projectId, docId, callback) { + const url = `${settings.apis.docstore.url}/project/${projectId}/doc/${docId}/deleted` + request.get({ url, timeout: TIMEOUT, json: true }, (err, res, body) => { + if (err) { + callback(err) + } else if (res.statusCode === 200) { + callback(null, body.deleted) + } else if (res.statusCode === 404) { + callback( + new Errors.NotFoundError({ + message: 'doc does not exist in project', + info: { projectId, docId }, + }) + ) + } else { + callback( + new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { projectId, docId } + ) + ) + } + }) +} + +function updateDoc(projectId, docId, lines, version, ranges, callback) { + const url = `${settings.apis.docstore.url}/project/${projectId}/doc/${docId}` + request.post( + { + url, + timeout: TIMEOUT, + json: { + lines, + version, + ranges, + }, + }, + (error, res, result) => { + if (error) { + return callback(error) + } + if (res.statusCode >= 200 && res.statusCode < 300) { + logger.log({ projectId, docId }, 'update doc in docstore url finished') + callback(null, result.modified, result.rev) + } else { + error = new OError( + `docstore api responded with non-success code: ${res.statusCode}`, + { projectId, docId } + ) + callback(error) + } + } + ) +} + +function archiveProject(projectId, callback) { + _operateOnProject(projectId, 'archive', callback) +} + +function unarchiveProject(projectId, callback) { + _operateOnProject(projectId, 'unarchive', callback) +} + +function destroyProject(projectId, callback) { + _operateOnProject(projectId, 'destroy', callback) +} + +function _operateOnProject(projectId, method, callback) { + const url = `${settings.apis.docstore.url}/project/${projectId}/${method}` + logger.log({ projectId }, `calling ${method} for project in docstore`) + // use default timeout for archiving/unarchiving/destroying + request.post(url, (err, res, docs) => { + if (err) { + OError.tag(err, `error calling ${method} project in docstore`, { + projectId, + }) + return callback(err) + } + + if (res.statusCode >= 200 && res.statusCode < 300) { + callback() + } else { + const error = new Error( + `docstore api responded with non-success code: ${res.statusCode}` + ) + logger.warn( + { err: error, projectId }, + `error calling ${method} project in docstore` + ) + callback(error) + } + }) +} + +module.exports = { + deleteDoc, + getAllDocs, + getAllDeletedDocs, + getAllRanges, + getDoc, + isDocDeleted, + updateDoc, + archiveProject, + unarchiveProject, + destroyProject, + promises: { + deleteDoc: promisify(deleteDoc), + getAllDocs: promisify(getAllDocs), + getAllDeletedDocs: promisify(getAllDeletedDocs), + getAllRanges: promisify(getAllRanges), + getDoc: promisifyMultiResult(getDoc, ['lines', 'rev', 'version', 'ranges']), + isDocDeleted: promisify(isDocDeleted), + updateDoc: promisifyMultiResult(updateDoc, ['modified', 'rev']), + archiveProject: promisify(archiveProject), + unarchiveProject: promisify(unarchiveProject), + destroyProject: promisify(destroyProject), }, -}) +} diff --git a/services/web/app/src/Features/Documents/DocumentController.js b/services/web/app/src/Features/Documents/DocumentController.js index f7eed9ac60..0d0e7e23af 100644 --- a/services/web/app/src/Features/Documents/DocumentController.js +++ b/services/web/app/src/Features/Documents/DocumentController.js @@ -1,17 +1,3 @@ -/* eslint-disable - camelcase, - node/handle-callback-err, - max-len, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * DS103: Rewrite code to no longer use __guard__ - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const ProjectGetter = require('../Project/ProjectGetter') const OError = require('@overleaf/o-error') const ProjectLocator = require('../Project/ProjectLocator') @@ -20,122 +6,109 @@ const ProjectEntityUpdateHandler = require('../Project/ProjectEntityUpdateHandle const logger = require('logger-sharelatex') const _ = require('lodash') -module.exports = { - getDocument(req, res, next) { - if (next == null) { - next = function (error) {} - } - const project_id = req.params.Project_id - const { doc_id } = req.params - const plain = - __guard__(req != null ? req.query : undefined, x => x.plain) === 'true' - return ProjectGetter.getProject( - project_id, - { rootFolder: true, overleaf: true }, - function (error, project) { - if (error != null) { - return next(error) - } - if (project == null) { - return res.sendStatus(404) - } - return ProjectLocator.findElement( - { project, element_id: doc_id, type: 'doc' }, - function (error, doc, path) { - if (error != null) { - OError.tag(error, 'error finding element for getDocument', { - doc_id, - project_id, - }) - return next(error) - } - return ProjectEntityHandler.getDoc( - project_id, - doc_id, - function (error, lines, rev, version, ranges) { - if (error != null) { - OError.tag( - error, - 'error finding doc contents for getDocument', - { - doc_id, - project_id, - } - ) - return next(error) - } - if (plain) { - res.type('text/plain') - return res.send(lines.join('\n')) - } else { - const projectHistoryId = _.get(project, 'overleaf.history.id') - const projectHistoryDisplay = _.get( - project, - 'overleaf.history.display' - ) - const sendToBothHistorySystems = _.get( - project, - 'overleaf.history.allowDowngrade' - ) - // if project has been switched but has 'allowDowngrade' set - // then leave projectHistoryType undefined to (temporarily) - // continue sending updates to both SL and full project history - const projectHistoryType = - projectHistoryDisplay && !sendToBothHistorySystems - ? 'project-history' - : undefined // for backwards compatibility, don't send anything if the project is still on track-changes - return res.json({ - lines, - version, - ranges, - pathname: path.fileSystem, - projectHistoryId, - projectHistoryType, - }) - } - } - ) +function getDocument(req, res, next) { + const { Project_id: projectId, doc_id: docId } = req.params + const plain = req.query.plain === 'true' + const peek = req.query.peek === 'true' + ProjectGetter.getProject( + projectId, + { rootFolder: true, overleaf: true }, + (error, project) => { + if (error) { + return next(error) + } + if (!project) { + return res.sendStatus(404) + } + ProjectLocator.findElement( + { project, element_id: docId, type: 'doc' }, + (error, doc, path) => { + if (error) { + OError.tag(error, 'error finding element for getDocument', { + docId, + projectId, + }) + return next(error) } - ) - } - ) - }, - - setDocument(req, res, next) { - if (next == null) { - next = function (error) {} - } - const project_id = req.params.Project_id - const { doc_id } = req.params - const { lines, version, ranges, lastUpdatedAt, lastUpdatedBy } = req.body - return ProjectEntityUpdateHandler.updateDocLines( - project_id, - doc_id, - lines, - version, - ranges, - lastUpdatedAt, - lastUpdatedBy, - function (error) { - if (error != null) { - OError.tag(error, 'error finding element for getDocument', { - doc_id, - project_id, - }) - return next(error) + ProjectEntityHandler.getDoc( + projectId, + docId, + { peek }, + (error, lines, rev, version, ranges) => { + if (error) { + OError.tag( + error, + 'error finding doc contents for getDocument', + { + docId, + projectId, + } + ) + return next(error) + } + if (plain) { + res.type('text/plain') + res.send(lines.join('\n')) + } else { + const projectHistoryId = _.get(project, 'overleaf.history.id') + const projectHistoryDisplay = _.get( + project, + 'overleaf.history.display' + ) + const sendToBothHistorySystems = _.get( + project, + 'overleaf.history.allowDowngrade' + ) + // if project has been switched but has 'allowDowngrade' set + // then leave projectHistoryType undefined to (temporarily) + // continue sending updates to both SL and full project history + const projectHistoryType = + projectHistoryDisplay && !sendToBothHistorySystems + ? 'project-history' + : undefined // for backwards compatibility, don't send anything if the project is still on track-changes + res.json({ + lines, + version, + ranges, + pathname: path.fileSystem, + projectHistoryId, + projectHistoryType, + }) + } + } + ) } - logger.log( - { doc_id, project_id }, - 'finished receiving set document request from api (docupdater)' - ) - return res.sendStatus(200) - } - ) - }, + ) + } + ) } -function __guard__(value, transform) { - return typeof value !== 'undefined' && value !== null - ? transform(value) - : undefined +function setDocument(req, res, next) { + const { Project_id: projectId, doc_id: docId } = req.params + const { lines, version, ranges, lastUpdatedAt, lastUpdatedBy } = req.body + ProjectEntityUpdateHandler.updateDocLines( + projectId, + docId, + lines, + version, + ranges, + lastUpdatedAt, + lastUpdatedBy, + error => { + if (error) { + OError.tag(error, 'error finding element for getDocument', { + docId, + projectId, + }) + return next(error) + } + logger.log( + { docId, projectId }, + 'finished receiving set document request from api (docupdater)' + ) + res.sendStatus(200) + } + ) } + +module.exports = { getDocument, setDocument } diff --git a/services/web/test/unit/src/Docstore/DocstoreManagerTests.js b/services/web/test/unit/src/Docstore/DocstoreManagerTests.js index 40fe409fee..e575ce7e0c 100644 --- a/services/web/test/unit/src/Docstore/DocstoreManagerTests.js +++ b/services/web/test/unit/src/Docstore/DocstoreManagerTests.js @@ -1,14 +1,3 @@ -/* eslint-disable - max-len, - no-return-assign, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const sinon = require('sinon') const modulePath = '../../../../app/src/Features/Docstore/DocstoreManager' const SandboxedModule = require('sandboxed-module') @@ -37,7 +26,7 @@ describe('DocstoreManager', function () { this.project_id = 'project-id-123' this.doc_id = 'doc-id-123' - return (this.callback = sinon.stub()) + this.callback = sinon.stub() }) describe('deleteDoc', function () { @@ -54,7 +43,7 @@ describe('DocstoreManager', function () { this.request.patch = sinon .stub() .callsArgWith(1, null, { statusCode: 204 }, '') - return this.DocstoreManager.deleteDoc( + this.DocstoreManager.deleteDoc( this.project_id, this.doc_id, 'wombat.tex', @@ -64,7 +53,7 @@ describe('DocstoreManager', function () { }) it('should delete the doc in the docstore api', function () { - return this.request.patch + this.request.patch .calledWith({ url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`, json: { deleted: true, deletedAt: new Date(), name: 'wombat.tex' }, @@ -74,7 +63,7 @@ describe('DocstoreManager', function () { }) it('should call the callback without an error', function () { - return this.callback.calledWith(null).should.equal(true) + this.callback.calledWith(null).should.equal(true) }) }) @@ -83,7 +72,7 @@ describe('DocstoreManager', function () { this.request.patch = sinon .stub() .callsArgWith(1, null, { statusCode: 500 }, '') - return this.DocstoreManager.deleteDoc( + this.DocstoreManager.deleteDoc( this.project_id, this.doc_id, 'main.tex', @@ -93,7 +82,7 @@ describe('DocstoreManager', function () { }) it('should call the callback with an error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Error) @@ -113,7 +102,7 @@ describe('DocstoreManager', function () { this.request.patch = sinon .stub() .callsArgWith(1, null, { statusCode: 404 }, '') - return this.DocstoreManager.deleteDoc( + this.DocstoreManager.deleteDoc( this.project_id, this.doc_id, 'main.tex', @@ -145,7 +134,7 @@ describe('DocstoreManager', function () { this.rev = 5 this.version = 42 this.ranges = { mock: 'ranges' } - return (this.modified = true) + this.modified = true }) describe('with a successful response code', function () { @@ -158,7 +147,7 @@ describe('DocstoreManager', function () { { statusCode: 204 }, { modified: this.modified, rev: this.rev } ) - return this.DocstoreManager.updateDoc( + this.DocstoreManager.updateDoc( this.project_id, this.doc_id, this.lines, @@ -169,7 +158,7 @@ describe('DocstoreManager', function () { }) it('should update the doc in the docstore api', function () { - return this.request.post + this.request.post .calledWith({ url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`, timeout: 30 * 1000, @@ -183,7 +172,7 @@ describe('DocstoreManager', function () { }) it('should call the callback with the modified status and revision', function () { - return this.callback + this.callback .calledWith(null, this.modified, this.rev) .should.equal(true) }) @@ -194,7 +183,7 @@ describe('DocstoreManager', function () { this.request.post = sinon .stub() .callsArgWith(1, null, { statusCode: 500 }, '') - return this.DocstoreManager.updateDoc( + this.DocstoreManager.updateDoc( this.project_id, this.doc_id, this.lines, @@ -205,7 +194,7 @@ describe('DocstoreManager', function () { }) it('should call the callback with an error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Error) @@ -223,12 +212,12 @@ describe('DocstoreManager', function () { describe('getDoc', function () { beforeEach(function () { - return (this.doc = { + this.doc = { lines: (this.lines = ['mock', 'doc', 'lines']), rev: (this.rev = 5), version: (this.version = 42), ranges: (this.ranges = { mock: 'ranges' }), - }) + } }) describe('with a successful response code', function () { @@ -236,25 +225,19 @@ describe('DocstoreManager', function () { this.request.get = sinon .stub() .callsArgWith(1, null, { statusCode: 204 }, this.doc) - return this.DocstoreManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) + this.DocstoreManager.getDoc(this.project_id, this.doc_id, this.callback) }) it('should get the doc from the docstore api', function () { - return this.request.get - .calledWith({ - url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`, - timeout: 30 * 1000, - json: true, - }) - .should.equal(true) + this.request.get.should.have.been.calledWith({ + url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`, + timeout: 30 * 1000, + json: true, + }) }) it('should call the callback with the lines, version and rev', function () { - return this.callback + this.callback .calledWith(null, this.lines, this.rev, this.version, this.ranges) .should.equal(true) }) @@ -265,15 +248,11 @@ describe('DocstoreManager', function () { this.request.get = sinon .stub() .callsArgWith(1, null, { statusCode: 500 }, '') - return this.DocstoreManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) + this.DocstoreManager.getDoc(this.project_id, this.doc_id, this.callback) }) it('should call the callback with an error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Error) @@ -293,7 +272,7 @@ describe('DocstoreManager', function () { this.request.get = sinon .stub() .callsArgWith(1, null, { statusCode: 204 }, this.doc) - return this.DocstoreManager.getDoc( + this.DocstoreManager.getDoc( this.project_id, this.doc_id, { include_deleted: true }, @@ -302,36 +281,53 @@ describe('DocstoreManager', function () { }) it('should get the doc from the docstore api (including deleted)', function () { - return this.request.get - .calledWith({ - url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}?include_deleted=true`, - timeout: 30 * 1000, - json: true, - }) - .should.equal(true) + this.request.get.should.have.been.calledWith({ + url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`, + qs: { include_deleted: 'true' }, + timeout: 30 * 1000, + json: true, + }) }) it('should call the callback with the lines, version and rev', function () { - return this.callback + this.callback .calledWith(null, this.lines, this.rev, this.version, this.ranges) .should.equal(true) }) }) + describe('with peek=true', function () { + beforeEach(function () { + this.request.get = sinon + .stub() + .callsArgWith(1, null, { statusCode: 204 }, this.doc) + this.DocstoreManager.getDoc( + this.project_id, + this.doc_id, + { peek: true }, + this.callback + ) + }) + + it('should call the docstore peek url', function () { + this.request.get.should.have.been.calledWith({ + url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}/peek`, + timeout: 30 * 1000, + json: true, + }) + }) + }) + describe('with a missing (404) response code', function () { beforeEach(function () { this.request.get = sinon .stub() .callsArgWith(1, null, { statusCode: 404 }, '') - return this.DocstoreManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) + this.DocstoreManager.getDoc(this.project_id, this.doc_id, this.callback) }) it('should call the callback with an error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Errors.NotFoundError) @@ -353,11 +349,11 @@ describe('DocstoreManager', function () { { statusCode: 204 }, (this.docs = [{ _id: 'mock-doc-id' }]) ) - return this.DocstoreManager.getAllDocs(this.project_id, this.callback) + this.DocstoreManager.getAllDocs(this.project_id, this.callback) }) it('should get all the project docs in the docstore api', function () { - return this.request.get + this.request.get .calledWith({ url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc`, timeout: 30 * 1000, @@ -367,7 +363,7 @@ describe('DocstoreManager', function () { }) it('should call the callback with the docs', function () { - return this.callback.calledWith(null, this.docs).should.equal(true) + this.callback.calledWith(null, this.docs).should.equal(true) }) }) @@ -376,11 +372,11 @@ describe('DocstoreManager', function () { this.request.get = sinon .stub() .callsArgWith(1, null, { statusCode: 500 }, '') - return this.DocstoreManager.getAllDocs(this.project_id, this.callback) + this.DocstoreManager.getAllDocs(this.project_id, this.callback) }) it('should call the callback with an error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Error) @@ -473,11 +469,11 @@ describe('DocstoreManager', function () { { statusCode: 204 }, (this.docs = [{ _id: 'mock-doc-id', ranges: 'mock-ranges' }]) ) - return this.DocstoreManager.getAllRanges(this.project_id, this.callback) + this.DocstoreManager.getAllRanges(this.project_id, this.callback) }) it('should get all the project doc ranges in the docstore api', function () { - return this.request.get + this.request.get .calledWith({ url: `${this.settings.apis.docstore.url}/project/${this.project_id}/ranges`, timeout: 30 * 1000, @@ -487,7 +483,7 @@ describe('DocstoreManager', function () { }) it('should call the callback with the docs', function () { - return this.callback.calledWith(null, this.docs).should.equal(true) + this.callback.calledWith(null, this.docs).should.equal(true) }) }) @@ -496,11 +492,11 @@ describe('DocstoreManager', function () { this.request.get = sinon .stub() .callsArgWith(1, null, { statusCode: 500 }, '') - return this.DocstoreManager.getAllRanges(this.project_id, this.callback) + this.DocstoreManager.getAllRanges(this.project_id, this.callback) }) it('should call the callback with an error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Error) @@ -522,14 +518,11 @@ describe('DocstoreManager', function () { this.request.post = sinon .stub() .callsArgWith(1, null, { statusCode: 204 }) - return this.DocstoreManager.archiveProject( - this.project_id, - this.callback - ) + this.DocstoreManager.archiveProject(this.project_id, this.callback) }) it('should call the callback', function () { - return this.callback.called.should.equal(true) + this.callback.called.should.equal(true) }) }) @@ -538,14 +531,11 @@ describe('DocstoreManager', function () { this.request.post = sinon .stub() .callsArgWith(1, null, { statusCode: 500 }) - return this.DocstoreManager.archiveProject( - this.project_id, - this.callback - ) + this.DocstoreManager.archiveProject(this.project_id, this.callback) }) it('should call the callback with an error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Error) @@ -567,14 +557,11 @@ describe('DocstoreManager', function () { this.request.post = sinon .stub() .callsArgWith(1, null, { statusCode: 204 }) - return this.DocstoreManager.unarchiveProject( - this.project_id, - this.callback - ) + this.DocstoreManager.unarchiveProject(this.project_id, this.callback) }) it('should call the callback', function () { - return this.callback.called.should.equal(true) + this.callback.called.should.equal(true) }) }) @@ -583,14 +570,11 @@ describe('DocstoreManager', function () { this.request.post = sinon .stub() .callsArgWith(1, null, { statusCode: 500 }) - return this.DocstoreManager.unarchiveProject( - this.project_id, - this.callback - ) + this.DocstoreManager.unarchiveProject(this.project_id, this.callback) }) it('should call the callback with an error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Error) @@ -612,14 +596,11 @@ describe('DocstoreManager', function () { this.request.post = sinon .stub() .callsArgWith(1, null, { statusCode: 204 }) - return this.DocstoreManager.destroyProject( - this.project_id, - this.callback - ) + this.DocstoreManager.destroyProject(this.project_id, this.callback) }) it('should call the callback', function () { - return this.callback.called.should.equal(true) + this.callback.called.should.equal(true) }) }) @@ -628,14 +609,11 @@ describe('DocstoreManager', function () { this.request.post = sinon .stub() .callsArgWith(1, null, { statusCode: 500 }) - return this.DocstoreManager.destroyProject( - this.project_id, - this.callback - ) + this.DocstoreManager.destroyProject(this.project_id, this.callback) }) it('should call the callback with an error', function () { - return this.callback + this.callback .calledWith( sinon.match .instanceOf(Error) diff --git a/services/web/test/unit/src/Documents/DocumentControllerTests.js b/services/web/test/unit/src/Documents/DocumentControllerTests.js index a2bf7082f9..a1b5da1b42 100644 --- a/services/web/test/unit/src/Documents/DocumentControllerTests.js +++ b/services/web/test/unit/src/Documents/DocumentControllerTests.js @@ -1,21 +1,7 @@ -/* eslint-disable - max-len, - no-return-assign, - no-unused-vars, -*/ -// TODO: This file was created by bulk-decaffeinate. -// Fix any style issues and re-enable lint. -/* - * decaffeinate suggestions: - * DS102: Remove unnecessary code created because of implicit returns - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ const sinon = require('sinon') -const { expect } = require('chai') const modulePath = '../../../../app/src/Features/Documents/DocumentController.js' const SandboxedModule = require('sandboxed-module') -const events = require('events') const MockRequest = require('../helpers/MockRequest') const MockResponse = require('../helpers/MockResponse') const Errors = require('../../../../app/src/Features/Errors/Errors') @@ -41,23 +27,23 @@ describe('DocumentController', function () { this.pathname = '/a/b/c/file.tex' this.lastUpdatedAt = new Date().getTime() this.lastUpdatedBy = 'fake-last-updater-id' - return (this.rev = 5) + this.rev = 5 }) describe('getDocument', function () { beforeEach(function () { - return (this.req.params = { + this.req.params = { Project_id: this.project_id, doc_id: this.doc_id, - }) + } }) describe('when the project exists without project history enabled', function () { beforeEach(function () { this.project = { _id: this.project_id } - return (this.ProjectGetter.getProject = sinon + this.ProjectGetter.getProject = sinon .stub() - .callsArgWith(2, null, this.project)) + .callsArgWith(2, null, this.project) }) describe('when the document exists', function () { @@ -68,29 +54,18 @@ describe('DocumentController', function () { .callsArgWith(1, null, this.doc, { fileSystem: this.pathname }) this.ProjectEntityHandler.getDoc = sinon .stub() - .callsArgWith( - 2, - null, - this.doc_lines, - this.rev, - this.version, - this.ranges - ) - return this.DocumentController.getDocument( - this.req, - this.res, - this.next - ) + .yields(null, this.doc_lines, this.rev, this.version, this.ranges) + this.DocumentController.getDocument(this.req, this.res, this.next) }) it('should get the project', function () { - return this.ProjectGetter.getProject + this.ProjectGetter.getProject .calledWith(this.project_id, { rootFolder: true, overleaf: true }) .should.equal(true) }) it('should get the pathname of the document', function () { - return this.ProjectLocator.findElement + this.ProjectLocator.findElement .calledWith({ project: this.project, element_id: this.doc_id, @@ -100,14 +75,14 @@ describe('DocumentController', function () { }) it('should get the document content', function () { - return this.ProjectEntityHandler.getDoc + this.ProjectEntityHandler.getDoc .calledWith(this.project_id, this.doc_id) .should.equal(true) }) it('should return the document data to the client as JSON', function () { this.res.type.should.equal('application/json') - return this.res.body.should.equal( + this.res.body.should.equal( JSON.stringify({ lines: this.doc_lines, version: this.version, @@ -123,15 +98,11 @@ describe('DocumentController', function () { this.ProjectLocator.findElement = sinon .stub() .callsArgWith(1, new Errors.NotFoundError('not found')) - return this.DocumentController.getDocument( - this.req, - this.res, - this.next - ) + this.DocumentController.getDocument(this.req, this.res, this.next) }) it('should call next with the NotFoundError', function () { - return this.next + this.next .calledWith(sinon.match.instanceOf(Errors.NotFoundError)) .should.equal(true) }) @@ -161,24 +132,13 @@ describe('DocumentController', function () { .callsArgWith(1, null, this.doc, { fileSystem: this.pathname }) this.ProjectEntityHandler.getDoc = sinon .stub() - .callsArgWith( - 2, - null, - this.doc_lines, - this.rev, - this.version, - this.ranges - ) - return this.DocumentController.getDocument( - this.req, - this.res, - this.next - ) + .yields(null, this.doc_lines, this.rev, this.version, this.ranges) + this.DocumentController.getDocument(this.req, this.res, this.next) }) it('should return the history id and display setting to the client as JSON', function () { this.res.type.should.equal('application/json') - return this.res.body.should.equal( + this.res.body.should.equal( JSON.stringify({ lines: this.doc_lines, version: this.version, @@ -215,14 +175,7 @@ describe('DocumentController', function () { .callsArgWith(1, null, this.doc, { fileSystem: this.pathname }) this.ProjectEntityHandler.getDoc = sinon .stub() - .callsArgWith( - 2, - null, - this.doc_lines, - this.rev, - this.version, - this.ranges - ) + .yields(null, this.doc_lines, this.rev, this.version, this.ranges) return this.DocumentController.getDocument( this.req, this.res, @@ -248,25 +201,21 @@ describe('DocumentController', function () { describe('when the project does not exist', function () { beforeEach(function () { this.ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, null) - return this.DocumentController.getDocument( - this.req, - this.res, - this.next - ) + this.DocumentController.getDocument(this.req, this.res, this.next) }) it('returns a 404', function () { - return this.res.statusCode.should.equal(404) + this.res.statusCode.should.equal(404) }) }) }) describe('setDocument', function () { beforeEach(function () { - return (this.req.params = { + this.req.params = { Project_id: this.project_id, doc_id: this.doc_id, - }) + } }) describe('when the document exists', function () { @@ -279,15 +228,11 @@ describe('DocumentController', function () { lastUpdatedAt: this.lastUpdatedAt, lastUpdatedBy: this.lastUpdatedBy, } - return this.DocumentController.setDocument( - this.req, - this.res, - this.next - ) + this.DocumentController.setDocument(this.req, this.res, this.next) }) it('should update the document in Mongo', function () { - return sinon.assert.calledWith( + sinon.assert.calledWith( this.ProjectEntityUpdateHandler.updateDocLines, this.project_id, this.doc_id, @@ -300,7 +245,7 @@ describe('DocumentController', function () { }) it('should return a successful response', function () { - return this.res.success.should.equal(true) + this.res.success.should.equal(true) }) }) @@ -310,15 +255,11 @@ describe('DocumentController', function () { .stub() .yields(new Errors.NotFoundError('document does not exist')) this.req.body = { lines: this.doc_lines } - return this.DocumentController.setDocument( - this.req, - this.res, - this.next - ) + this.DocumentController.setDocument(this.req, this.res, this.next) }) it('should call next with the NotFoundError', function () { - return this.next + this.next .calledWith(sinon.match.instanceOf(Errors.NotFoundError)) .should.equal(true) })