From 7a0b40a4bf05e4c06647c64f49af776def2a3da9 Mon Sep 17 00:00:00 2001 From: Eric Mc Sween Date: Tue, 30 Nov 2021 08:26:20 -0500 Subject: [PATCH] Merge pull request #5941 from overleaf/em-cleanup-redis-manager Decaf cleanup docupdater RedisManager and DocumentManager GitOrigin-RevId: 6a9634d1882a3a7328cac8cd88537dd9d204b281 --- services/document-updater/.eslintrc | 2 +- .../app/js/DocumentManager.js | 696 +++++++----------- .../document-updater/app/js/RedisManager.js | 625 +++++++--------- .../unit/js/RedisManager/RedisManagerTests.js | 572 +++++++------- 4 files changed, 828 insertions(+), 1067 deletions(-) diff --git a/services/document-updater/.eslintrc b/services/document-updater/.eslintrc index a35d42c503..f48533f9e2 100644 --- a/services/document-updater/.eslintrc +++ b/services/document-updater/.eslintrc @@ -8,7 +8,7 @@ "prettier" ], "parserOptions": { - "ecmaVersion": 2018 + "ecmaVersion": 2020 }, "plugins": [ "mocha", diff --git a/services/document-updater/app/js/DocumentManager.js b/services/document-updater/app/js/DocumentManager.js index 45d3aa80df..8ff285c5ff 100644 --- a/services/document-updater/app/js/DocumentManager.js +++ b/services/document-updater/app/js/DocumentManager.js @@ -1,16 +1,3 @@ -/* eslint-disable - camelcase, - 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 - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ let DocumentManager const RedisManager = require('./RedisManager') const ProjectHistoryRedisManager = require('./ProjectHistoryRedisManager') @@ -19,28 +6,23 @@ const DiffCodec = require('./DiffCodec') const logger = require('@overleaf/logger') const Metrics = require('./Metrics') const HistoryManager = require('./HistoryManager') -const RealTimeRedisManager = require('./RealTimeRedisManager') const Errors = require('./Errors') const RangesManager = require('./RangesManager') -const async = require('async') const MAX_UNFLUSHED_AGE = 300 * 1000 // 5 mins, document should be flushed to mongo this time after a change module.exports = DocumentManager = { - getDoc(project_id, doc_id, _callback) { - if (_callback == null) { - _callback = function () {} - } + getDoc(projectId, docId, _callback) { const timer = new Metrics.Timer('docManager.getDoc') - const callback = function (...args) { + const callback = (...args) => { timer.done() - return _callback(...Array.from(args || [])) + _callback(...args) } - return RedisManager.getDoc( - project_id, - doc_id, - function ( + RedisManager.getDoc( + projectId, + docId, + ( error, lines, version, @@ -48,19 +30,19 @@ module.exports = DocumentManager = { pathname, projectHistoryId, unflushedTime - ) { - if (error != null) { + ) => { + if (error) { return callback(error) } if (lines == null || version == null) { logger.debug( - { project_id, doc_id }, + { projectId, docId }, 'doc not in redis so getting from persistence API' ) - return PersistenceManager.getDoc( - project_id, - doc_id, - function ( + PersistenceManager.getDoc( + projectId, + docId, + ( error, lines, version, @@ -68,14 +50,14 @@ module.exports = DocumentManager = { pathname, projectHistoryId, projectHistoryType - ) { - if (error != null) { + ) => { + if (error) { return callback(error) } logger.debug( { - project_id, - doc_id, + projectId, + docId, lines, version, pathname, @@ -84,26 +66,26 @@ module.exports = DocumentManager = { }, 'got doc from persistence API' ) - return RedisManager.putDocInMemory( - project_id, - doc_id, + RedisManager.putDocInMemory( + projectId, + docId, lines, version, ranges, pathname, projectHistoryId, - function (error) { - if (error != null) { + error => { + if (error) { return callback(error) } - return RedisManager.setHistoryType( - doc_id, + RedisManager.setHistoryType( + docId, projectHistoryType, - function (error) { - if (error != null) { + error => { + if (error) { return callback(error) } - return callback( + callback( null, lines, version, @@ -120,7 +102,7 @@ module.exports = DocumentManager = { } ) } else { - return callback( + callback( null, lines, version, @@ -135,43 +117,32 @@ module.exports = DocumentManager = { ) }, - getDocAndRecentOps(project_id, doc_id, fromVersion, _callback) { - if (_callback == null) { - _callback = function () {} - } + getDocAndRecentOps(projectId, docId, fromVersion, _callback) { const timer = new Metrics.Timer('docManager.getDocAndRecentOps') - const callback = function (...args) { + const callback = (...args) => { timer.done() - return _callback(...Array.from(args || [])) + _callback(...args) } - return DocumentManager.getDoc( - project_id, - doc_id, - function (error, lines, version, ranges, pathname, projectHistoryId) { - if (error != null) { + DocumentManager.getDoc( + projectId, + docId, + (error, lines, version, ranges, pathname, projectHistoryId) => { + if (error) { return callback(error) } if (fromVersion === -1) { - return callback( - null, - lines, - version, - [], - ranges, - pathname, - projectHistoryId - ) + callback(null, lines, version, [], ranges, pathname, projectHistoryId) } else { - return RedisManager.getPreviousDocOps( - doc_id, + RedisManager.getPreviousDocOps( + docId, fromVersion, version, - function (error, ops) { - if (error != null) { + (error, ops) => { + if (error) { return callback(error) } - return callback( + callback( null, lines, version, @@ -187,14 +158,11 @@ module.exports = DocumentManager = { ) }, - setDoc(project_id, doc_id, newLines, source, user_id, undoing, _callback) { - if (_callback == null) { - _callback = function () {} - } + setDoc(projectId, docId, newLines, source, userId, undoing, _callback) { const timer = new Metrics.Timer('docManager.setDoc') - const callback = function (...args) { + const callback = (...args) => { timer.done() - return _callback(...Array.from(args || [])) + _callback(...args) } if (newLines == null) { @@ -202,10 +170,10 @@ module.exports = DocumentManager = { } const UpdateManager = require('./UpdateManager') - return DocumentManager.getDoc( - project_id, - doc_id, - function ( + DocumentManager.getDoc( + projectId, + docId, + ( error, oldLines, version, @@ -214,8 +182,8 @@ module.exports = DocumentManager = { projectHistoryId, unflushedTime, alreadyLoaded - ) { - if (error != null) { + ) => { + if (error) { return callback(error) } @@ -225,99 +193,78 @@ module.exports = DocumentManager = { oldLines[0].text != null ) { logger.debug( - { doc_id, project_id, oldLines, newLines }, + { docId, projectId, oldLines, newLines }, 'document is JSON so not updating' ) return callback(null) } logger.debug( - { doc_id, project_id, oldLines, newLines }, + { docId, projectId, oldLines, newLines }, 'setting a document via http' ) - return DiffCodec.diffAsShareJsOp( - oldLines, - newLines, - function (error, op) { - if (error != null) { + DiffCodec.diffAsShareJsOp(oldLines, newLines, (error, op) => { + if (error) { + return callback(error) + } + if (undoing) { + for (const o of op || []) { + o.u = true + } // Turn on undo flag for each op for track changes + } + const update = { + doc: docId, + op, + v: version, + meta: { + type: 'external', + source, + user_id: userId, + }, + } + UpdateManager.applyUpdate(projectId, docId, update, error => { + if (error) { return callback(error) } - if (undoing) { - for (const o of Array.from(op || [])) { - o.u = true - } // Turn on undo flag for each op for track changes - } - const update = { - doc: doc_id, - op, - v: version, - meta: { - type: 'external', - source, - user_id, - }, - } - return UpdateManager.applyUpdate( - project_id, - doc_id, - update, - function (error) { - if (error != null) { + // If the document was loaded already, then someone has it open + // in a project, and the usual flushing mechanism will happen. + // Otherwise we should remove it immediately since nothing else + // is using it. + if (alreadyLoaded) { + DocumentManager.flushDocIfLoaded(projectId, docId, error => { + if (error) { return callback(error) } - // If the document was loaded already, then someone has it open - // in a project, and the usual flushing mechanism will happen. - // Otherwise we should remove it immediately since nothing else - // is using it. - if (alreadyLoaded) { - return DocumentManager.flushDocIfLoaded( - project_id, - doc_id, - function (error) { - if (error != null) { - return callback(error) - } - return callback(null) - } - ) - } else { - return DocumentManager.flushAndDeleteDoc( - project_id, - doc_id, - {}, - function (error) { - // There is no harm in flushing project history if the previous - // call failed and sometimes it is required - HistoryManager.flushProjectChangesAsync(project_id) + callback(null) + }) + } else { + DocumentManager.flushAndDeleteDoc(projectId, docId, {}, error => { + // There is no harm in flushing project history if the previous + // call failed and sometimes it is required + HistoryManager.flushProjectChangesAsync(projectId) - if (error != null) { - return callback(error) - } - return callback(null) - } - ) + if (error) { + return callback(error) } - } - ) - } - ) + callback(null) + }) + } + }) + }) } ) }, - flushDocIfLoaded(project_id, doc_id, _callback) { - if (_callback == null) { - _callback = function () {} - } + flushDocIfLoaded(projectId, docId, _callback) { const timer = new Metrics.Timer('docManager.flushDocIfLoaded') - const callback = function (...args) { + const callback = (...args) => { timer.done() - return _callback(...Array.from(args || [])) + _callback(...args) } - return RedisManager.getDoc( - project_id, - doc_id, - function ( + RedisManager.getDoc( + projectId, + docId, + ( error, lines, version, @@ -327,31 +274,32 @@ module.exports = DocumentManager = { unflushedTime, lastUpdatedAt, lastUpdatedBy - ) { - if (error != null) { + ) => { + if (error) { return callback(error) } if (lines == null || version == null) { logger.debug( - { project_id, doc_id }, + { projectId, docId }, 'doc is not loaded so not flushing' ) - return callback(null) // TODO: return a flag to bail out, as we go on to remove doc from memory? + // TODO: return a flag to bail out, as we go on to remove doc from memory? + callback(null) } else { - logger.debug({ project_id, doc_id, version }, 'flushing doc') - return PersistenceManager.setDoc( - project_id, - doc_id, + logger.debug({ projectId, docId, version }, 'flushing doc') + PersistenceManager.setDoc( + projectId, + docId, lines, version, ranges, lastUpdatedAt, lastUpdatedBy, - function (error) { - if (error != null) { + error => { + if (error) { return callback(error) } - return RedisManager.clearUnflushedTime(doc_id, callback) + RedisManager.clearUnflushedTime(docId, callback) } ) } @@ -359,176 +307,148 @@ module.exports = DocumentManager = { ) }, - flushAndDeleteDoc(project_id, doc_id, options, _callback) { + flushAndDeleteDoc(projectId, docId, options, _callback) { const timer = new Metrics.Timer('docManager.flushAndDeleteDoc') - const callback = function (...args) { + const callback = (...args) => { timer.done() - return _callback(...Array.from(args || [])) + _callback(...args) } - return DocumentManager.flushDocIfLoaded( - project_id, - doc_id, - function (error) { - if (error != null) { - if (options.ignoreFlushErrors) { - logger.warn( - { project_id, doc_id, err: error }, - 'ignoring flush error while deleting document' - ) - } else { - return callback(error) - } + DocumentManager.flushDocIfLoaded(projectId, docId, error => { + if (error) { + if (options.ignoreFlushErrors) { + logger.warn( + { projectId, docId, err: error }, + 'ignoring flush error while deleting document' + ) + } else { + return callback(error) } - - // Flush in the background since it requires a http request - HistoryManager.flushDocChangesAsync(project_id, doc_id) - - return RedisManager.removeDocFromMemory( - project_id, - doc_id, - function (error) { - if (error != null) { - return callback(error) - } - return callback(null) - } - ) } - ) + + // Flush in the background since it requires a http request + HistoryManager.flushDocChangesAsync(projectId, docId) + + RedisManager.removeDocFromMemory(projectId, docId, error => { + if (error) { + return callback(error) + } + callback(null) + }) + }) }, - acceptChanges(project_id, doc_id, change_ids, _callback) { - if (change_ids == null) { - change_ids = [] - } - if (_callback == null) { - _callback = function () {} + acceptChanges(projectId, docId, changeIds, _callback) { + if (changeIds == null) { + changeIds = [] } const timer = new Metrics.Timer('docManager.acceptChanges') - const callback = function (...args) { + const callback = (...args) => { timer.done() - return _callback(...Array.from(args || [])) + _callback(...args) } - return DocumentManager.getDoc( - project_id, - doc_id, - function (error, lines, version, ranges) { - if (error != null) { + DocumentManager.getDoc( + projectId, + docId, + (error, lines, version, ranges) => { + if (error) { return callback(error) } if (lines == null || version == null) { return callback( - new Errors.NotFoundError(`document not found: ${doc_id}`) + new Errors.NotFoundError(`document not found: ${docId}`) ) } - return RangesManager.acceptChanges( - change_ids, - ranges, - function (error, new_ranges) { - if (error != null) { - return callback(error) - } - return RedisManager.updateDocument( - project_id, - doc_id, - lines, - version, - [], - new_ranges, - {}, - function (error) { - if (error != null) { - return callback(error) - } - return callback() - } - ) + RangesManager.acceptChanges(changeIds, ranges, (error, newRanges) => { + if (error) { + return callback(error) } - ) + RedisManager.updateDocument( + projectId, + docId, + lines, + version, + [], + newRanges, + {}, + error => { + if (error) { + return callback(error) + } + callback() + } + ) + }) } ) }, - deleteComment(project_id, doc_id, comment_id, _callback) { - if (_callback == null) { - _callback = function () {} - } + deleteComment(projectId, docId, commentId, _callback) { const timer = new Metrics.Timer('docManager.deleteComment') - const callback = function (...args) { + const callback = (...args) => { timer.done() - return _callback(...Array.from(args || [])) + _callback(...args) } - return DocumentManager.getDoc( - project_id, - doc_id, - function (error, lines, version, ranges) { - if (error != null) { + DocumentManager.getDoc( + projectId, + docId, + (error, lines, version, ranges) => { + if (error) { return callback(error) } if (lines == null || version == null) { return callback( - new Errors.NotFoundError(`document not found: ${doc_id}`) + new Errors.NotFoundError(`document not found: ${docId}`) ) } - return RangesManager.deleteComment( - comment_id, - ranges, - function (error, new_ranges) { - if (error != null) { - return callback(error) - } - return RedisManager.updateDocument( - project_id, - doc_id, - lines, - version, - [], - new_ranges, - {}, - function (error) { - if (error != null) { - return callback(error) - } - return callback() - } - ) + RangesManager.deleteComment(commentId, ranges, (error, newRanges) => { + if (error) { + return callback(error) } - ) + RedisManager.updateDocument( + projectId, + docId, + lines, + version, + [], + newRanges, + {}, + error => { + if (error) { + return callback(error) + } + callback() + } + ) + }) } ) }, - renameDoc(project_id, doc_id, user_id, update, projectHistoryId, _callback) { - if (_callback == null) { - _callback = function () {} - } + renameDoc(projectId, docId, userId, update, projectHistoryId, _callback) { const timer = new Metrics.Timer('docManager.updateProject') - const callback = function (...args) { + const callback = (...args) => { timer.done() - return _callback(...Array.from(args || [])) + _callback(...args) } - return RedisManager.renameDoc( - project_id, - doc_id, - user_id, + RedisManager.renameDoc( + projectId, + docId, + userId, update, projectHistoryId, callback ) }, - getDocAndFlushIfOld(project_id, doc_id, callback) { - if (callback == null) { - callback = function () {} - } - return DocumentManager.getDoc( - project_id, - doc_id, - function ( + getDocAndFlushIfOld(projectId, docId, callback) { + DocumentManager.getDoc( + projectId, + docId, + ( error, lines, version, @@ -537,8 +457,8 @@ module.exports = DocumentManager = { projectHistoryId, unflushedTime, alreadyLoaded - ) { - if (error != null) { + ) => { + if (error) { return callback(error) } // if doc was already loaded see if it needs to be flushed @@ -547,65 +467,54 @@ module.exports = DocumentManager = { unflushedTime != null && Date.now() - unflushedTime > MAX_UNFLUSHED_AGE ) { - return DocumentManager.flushDocIfLoaded( - project_id, - doc_id, - function (error) { - if (error != null) { - return callback(error) - } - return callback(null, lines, version) + DocumentManager.flushDocIfLoaded(projectId, docId, error => { + if (error) { + return callback(error) } - ) + callback(null, lines, version) + }) } else { - return callback(null, lines, version) + callback(null, lines, version) } } ) }, - resyncDocContents(project_id, doc_id, path, callback) { - logger.debug({ project_id, doc_id, path }, 'start resyncing doc contents') - return RedisManager.getDoc( - project_id, - doc_id, - function (error, lines, version, ranges, pathname, projectHistoryId) { - if (error != null) { + resyncDocContents(projectId, docId, path, callback) { + logger.debug({ projectId, docId, path }, 'start resyncing doc contents') + RedisManager.getDoc( + projectId, + docId, + (error, lines, version, ranges, pathname, projectHistoryId) => { + if (error) { return callback(error) } - // To avoid issues where the same doc_id appears with different paths, + // To avoid issues where the same docId appears with different paths, // we use the path from the resyncProjectStructure update. If we used // the path from the getDoc call to web then the two occurences of the - // doc_id would map to the same path, and this would be rejected by + // docId would map to the same path, and this would be rejected by // project-history as an unexpected resyncDocContent update. if (lines == null || version == null) { logger.debug( - { project_id, doc_id }, + { projectId, docId }, 'resyncing doc contents - not found in redis - retrieving from web' ) - return PersistenceManager.getDoc( - project_id, - doc_id, + PersistenceManager.getDoc( + projectId, + docId, { peek: true }, - function ( - error, - lines, - version, - ranges, - pathname, - projectHistoryId - ) { - if (error != null) { + (error, lines, version, ranges, pathname, projectHistoryId) => { + if (error) { logger.error( - { project_id, doc_id, getDocError: error }, + { projectId, docId, getDocError: error }, 'resyncing doc contents - error retrieving from web' ) return callback(error) } - return ProjectHistoryRedisManager.queueResyncDocContent( - project_id, + ProjectHistoryRedisManager.queueResyncDocContent( + projectId, projectHistoryId, - doc_id, + docId, lines, version, path, // use the path from the resyncProjectStructure update @@ -615,13 +524,13 @@ module.exports = DocumentManager = { ) } else { logger.debug( - { project_id, doc_id }, + { projectId, docId }, 'resyncing doc contents - doc in redis - will queue in redis' ) - return ProjectHistoryRedisManager.queueResyncDocContent( - project_id, + ProjectHistoryRedisManager.queueResyncDocContent( + projectId, projectHistoryId, - doc_id, + docId, lines, version, path, // use the path from the resyncProjectStructure update @@ -632,155 +541,120 @@ module.exports = DocumentManager = { ) }, - getDocWithLock(project_id, doc_id, callback) { - if (callback == null) { - callback = function () {} - } + getDocWithLock(projectId, docId, callback) { const UpdateManager = require('./UpdateManager') - return UpdateManager.lockUpdatesAndDo( + UpdateManager.lockUpdatesAndDo( DocumentManager.getDoc, - project_id, - doc_id, + projectId, + docId, callback ) }, - getDocAndRecentOpsWithLock(project_id, doc_id, fromVersion, callback) { - if (callback == null) { - callback = function () {} - } + getDocAndRecentOpsWithLock(projectId, docId, fromVersion, callback) { const UpdateManager = require('./UpdateManager') - return UpdateManager.lockUpdatesAndDo( + UpdateManager.lockUpdatesAndDo( DocumentManager.getDocAndRecentOps, - project_id, - doc_id, + projectId, + docId, fromVersion, callback ) }, - getDocAndFlushIfOldWithLock(project_id, doc_id, callback) { - if (callback == null) { - callback = function () {} - } + getDocAndFlushIfOldWithLock(projectId, docId, callback) { const UpdateManager = require('./UpdateManager') - return UpdateManager.lockUpdatesAndDo( + UpdateManager.lockUpdatesAndDo( DocumentManager.getDocAndFlushIfOld, - project_id, - doc_id, + projectId, + docId, callback ) }, - setDocWithLock( - project_id, - doc_id, - lines, - source, - user_id, - undoing, - callback - ) { - if (callback == null) { - callback = function () {} - } + setDocWithLock(projectId, docId, lines, source, userId, undoing, callback) { const UpdateManager = require('./UpdateManager') - return UpdateManager.lockUpdatesAndDo( + UpdateManager.lockUpdatesAndDo( DocumentManager.setDoc, - project_id, - doc_id, + projectId, + docId, lines, source, - user_id, + userId, undoing, callback ) }, - flushDocIfLoadedWithLock(project_id, doc_id, callback) { - if (callback == null) { - callback = function () {} - } + flushDocIfLoadedWithLock(projectId, docId, callback) { const UpdateManager = require('./UpdateManager') - return UpdateManager.lockUpdatesAndDo( + UpdateManager.lockUpdatesAndDo( DocumentManager.flushDocIfLoaded, - project_id, - doc_id, + projectId, + docId, callback ) }, - flushAndDeleteDocWithLock(project_id, doc_id, options, callback) { + flushAndDeleteDocWithLock(projectId, docId, options, callback) { const UpdateManager = require('./UpdateManager') - return UpdateManager.lockUpdatesAndDo( + UpdateManager.lockUpdatesAndDo( DocumentManager.flushAndDeleteDoc, - project_id, - doc_id, + projectId, + docId, options, callback ) }, - acceptChangesWithLock(project_id, doc_id, change_ids, callback) { - if (callback == null) { - callback = function () {} - } + acceptChangesWithLock(projectId, docId, changeIds, callback) { const UpdateManager = require('./UpdateManager') - return UpdateManager.lockUpdatesAndDo( + UpdateManager.lockUpdatesAndDo( DocumentManager.acceptChanges, - project_id, - doc_id, - change_ids, + projectId, + docId, + changeIds, callback ) }, - deleteCommentWithLock(project_id, doc_id, thread_id, callback) { - if (callback == null) { - callback = function () {} - } + deleteCommentWithLock(projectId, docId, threadId, callback) { const UpdateManager = require('./UpdateManager') - return UpdateManager.lockUpdatesAndDo( + UpdateManager.lockUpdatesAndDo( DocumentManager.deleteComment, - project_id, - doc_id, - thread_id, + projectId, + docId, + threadId, callback ) }, renameDocWithLock( - project_id, - doc_id, - user_id, + projectId, + docId, + userId, update, projectHistoryId, callback ) { - if (callback == null) { - callback = function () {} - } const UpdateManager = require('./UpdateManager') - return UpdateManager.lockUpdatesAndDo( + UpdateManager.lockUpdatesAndDo( DocumentManager.renameDoc, - project_id, - doc_id, - user_id, + projectId, + docId, + userId, update, projectHistoryId, callback ) }, - resyncDocContentsWithLock(project_id, doc_id, path, callback) { - if (callback == null) { - callback = function () {} - } + resyncDocContentsWithLock(projectId, docId, path, callback) { const UpdateManager = require('./UpdateManager') - return UpdateManager.lockUpdatesAndDo( + UpdateManager.lockUpdatesAndDo( DocumentManager.resyncDocContents, - project_id, - doc_id, + projectId, + docId, path, callback ) diff --git a/services/document-updater/app/js/RedisManager.js b/services/document-updater/app/js/RedisManager.js index d392443baa..a5bab3c98b 100644 --- a/services/document-updater/app/js/RedisManager.js +++ b/services/document-updater/app/js/RedisManager.js @@ -1,17 +1,3 @@ -/* eslint-disable - camelcase, -*/ -// 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 - * DS103: Rewrite code to no longer use __guard__ - * DS201: Simplify complex destructure assignments - * DS207: Consider shorter variations of null checks - * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md - */ let RedisManager const Settings = require('@overleaf/settings') const rclient = require('@overleaf/redis-wrapper').createClient( @@ -34,11 +20,7 @@ const MAX_REDIS_REQUEST_LENGTH = 5000 // 5 seconds // Make times easy to read const minutes = 60 // seconds for Redis expire -const logHashErrors = - Settings.documentupdater != null - ? Settings.documentupdater.logHashErrors - : undefined -const logHashReadErrors = logHashErrors != null ? logHashErrors.read : undefined +const logHashReadErrors = Settings.documentupdater?.logHashErrors?.read const MEGABYTES = 1024 * 1024 const MAX_RANGES_SIZE = 3 * MEGABYTES @@ -50,8 +32,8 @@ module.exports = RedisManager = { rclient, putDocInMemory( - project_id, - doc_id, + projectId, + docId, docLines, version, ranges, @@ -60,9 +42,9 @@ module.exports = RedisManager = { _callback ) { const timer = new metrics.Timer('redis.put-doc') - const callback = function (error) { + const callback = error => { timer.done() - return _callback(error) + _callback(error) } const docLinesArray = docLines docLines = JSON.stringify(docLines) @@ -70,7 +52,7 @@ module.exports = RedisManager = { const error = new Error('null bytes found in doc lines') // this check was added to catch memory corruption in JSON.stringify. // It sometimes returned null bytes at the end of the string. - logger.error({ err: error, doc_id, docLines }, error.message) + logger.error({ err: error, docId, docLines }, error.message) return callback(error) } // Do an optimised size check on the docLines using the serialised @@ -79,151 +61,146 @@ module.exports = RedisManager = { if (docIsTooLarge(sizeBound, docLinesArray, Settings.max_doc_length)) { const docSize = docLines.length const err = new Error('blocking doc insert into redis: doc is too large') - logger.error({ project_id, doc_id, err, docSize }, err.message) + logger.error({ projectId, docId, err, docSize }, err.message) return callback(err) } const docHash = RedisManager._computeHash(docLines) // record bytes sent to redis metrics.summary('redis.docLines', docLines.length, { status: 'set' }) logger.debug( - { project_id, doc_id, version, docHash, pathname, projectHistoryId }, + { projectId, docId, version, docHash, pathname, projectHistoryId }, 'putting doc in redis' ) - return RedisManager._serializeRanges(ranges, function (error, ranges) { - if (error != null) { - logger.error({ err: error, doc_id, project_id }, error.message) + RedisManager._serializeRanges(ranges, (error, ranges) => { + if (error) { + logger.error({ err: error, docId, projectId }, error.message) return callback(error) } // update docsInProject set before writing doc contents - rclient.sadd(keys.docsInProject({ project_id }), doc_id, error => { - if (error) return callback(error) + rclient.sadd( + keys.docsInProject({ project_id: projectId }), + docId, + error => { + if (error) return callback(error) - if (!pathname) { - metrics.inc('pathname', 1, { - path: 'RedisManager.setDoc', - status: pathname === '' ? 'zero-length' : 'undefined', - }) + if (!pathname) { + metrics.inc('pathname', 1, { + path: 'RedisManager.setDoc', + status: pathname === '' ? 'zero-length' : 'undefined', + }) + } + + rclient.mset( + { + [keys.docLines({ doc_id: docId })]: docLines, + [keys.projectKey({ doc_id: docId })]: projectId, + [keys.docVersion({ doc_id: docId })]: version, + [keys.docHash({ doc_id: docId })]: docHash, + [keys.ranges({ doc_id: docId })]: ranges, + [keys.pathname({ doc_id: docId })]: pathname, + [keys.projectHistoryId({ doc_id: docId })]: projectHistoryId, + }, + callback + ) } - - rclient.mset( - { - [keys.docLines({ doc_id })]: docLines, - [keys.projectKey({ doc_id })]: project_id, - [keys.docVersion({ doc_id })]: version, - [keys.docHash({ doc_id })]: docHash, - [keys.ranges({ doc_id })]: ranges, - [keys.pathname({ doc_id })]: pathname, - [keys.projectHistoryId({ doc_id })]: projectHistoryId, - }, - callback - ) - }) + ) }) }, - removeDocFromMemory(project_id, doc_id, _callback) { - logger.debug({ project_id, doc_id }, 'removing doc from redis') - const callback = function (err) { - if (err != null) { - logger.err({ project_id, doc_id, err }, 'error removing doc from redis') - return _callback(err) + removeDocFromMemory(projectId, docId, _callback) { + logger.debug({ projectId, docId }, 'removing doc from redis') + const callback = err => { + if (err) { + logger.err({ projectId, docId, err }, 'error removing doc from redis') + _callback(err) } else { - logger.debug({ project_id, doc_id }, 'removed doc from redis') - return _callback() + logger.debug({ projectId, docId }, 'removed doc from redis') + _callback() } } let multi = rclient.multi() - multi.strlen(keys.docLines({ doc_id })) + multi.strlen(keys.docLines({ doc_id: docId })) multi.del( - keys.docLines({ doc_id }), - keys.projectKey({ doc_id }), - keys.docVersion({ doc_id }), - keys.docHash({ doc_id }), - keys.ranges({ doc_id }), - keys.pathname({ doc_id }), - keys.projectHistoryId({ doc_id }), - keys.projectHistoryType({ doc_id }), - keys.unflushedTime({ doc_id }), - keys.lastUpdatedAt({ doc_id }), - keys.lastUpdatedBy({ doc_id }) + keys.docLines({ doc_id: docId }), + keys.projectKey({ doc_id: docId }), + keys.docVersion({ doc_id: docId }), + keys.docHash({ doc_id: docId }), + keys.ranges({ doc_id: docId }), + keys.pathname({ doc_id: docId }), + keys.projectHistoryId({ doc_id: docId }), + keys.projectHistoryType({ doc_id: docId }), + keys.unflushedTime({ doc_id: docId }), + keys.lastUpdatedAt({ doc_id: docId }), + keys.lastUpdatedBy({ doc_id: docId }) ) - return multi.exec(function (error, response) { - if (error != null) { + multi.exec((error, response) => { + if (error) { return callback(error) } - const length = response != null ? response[0] : undefined + const length = response?.[0] if (length > 0) { // record bytes freed in redis metrics.summary('redis.docLines', length, { status: 'del' }) } multi = rclient.multi() - multi.srem(keys.docsInProject({ project_id }), doc_id) - multi.del(keys.projectState({ project_id })) - return multi.exec(callback) + multi.srem(keys.docsInProject({ project_id: projectId }), docId) + multi.del(keys.projectState({ project_id: projectId })) + multi.exec(callback) }) }, - checkOrSetProjectState(project_id, newState, callback) { - if (callback == null) { - callback = function () {} - } + checkOrSetProjectState(projectId, newState, callback) { const multi = rclient.multi() - multi.getset(keys.projectState({ project_id }), newState) - multi.expire(keys.projectState({ project_id }), 30 * minutes) - return multi.exec(function (error, response) { - if (error != null) { + multi.getset(keys.projectState({ project_id: projectId }), newState) + multi.expire(keys.projectState({ project_id: projectId }), 30 * minutes) + multi.exec((error, response) => { + if (error) { return callback(error) } logger.debug( - { project_id, newState, oldState: response[0] }, + { projectId, newState, oldState: response[0] }, 'checking project state' ) - return callback(null, response[0] !== newState) + callback(null, response[0] !== newState) }) }, - clearProjectState(project_id, callback) { - if (callback == null) { - callback = function () {} - } - return rclient.del(keys.projectState({ project_id }), callback) + clearProjectState(projectId, callback) { + rclient.del(keys.projectState({ project_id: projectId }), callback) }, - getDoc(project_id, doc_id, callback) { - if (callback == null) { - callback = function () {} - } + getDoc(projectId, docId, callback) { const timer = new metrics.Timer('redis.get-doc') const collectKeys = [ - keys.docLines({ doc_id }), - keys.docVersion({ doc_id }), - keys.docHash({ doc_id }), - keys.projectKey({ doc_id }), - keys.ranges({ doc_id }), - keys.pathname({ doc_id }), - keys.projectHistoryId({ doc_id }), - keys.unflushedTime({ doc_id }), - keys.lastUpdatedAt({ doc_id }), - keys.lastUpdatedBy({ doc_id }), + keys.docLines({ doc_id: docId }), + keys.docVersion({ doc_id: docId }), + keys.docHash({ doc_id: docId }), + keys.projectKey({ doc_id: docId }), + keys.ranges({ doc_id: docId }), + keys.pathname({ doc_id: docId }), + keys.projectHistoryId({ doc_id: docId }), + keys.unflushedTime({ doc_id: docId }), + keys.lastUpdatedAt({ doc_id: docId }), + keys.lastUpdatedBy({ doc_id: docId }), ] - rclient.mget(...collectKeys, (error, ...rest) => { + rclient.mget(...collectKeys, (error, result) => { + if (error) { + return callback(error) + } let [ docLines, version, storedHash, - doc_project_id, + docProjectId, ranges, pathname, projectHistoryId, unflushedTime, lastUpdatedAt, lastUpdatedBy, - ] = Array.from(rest[0]) + ] = result const timeSpan = timer.done() - if (error != null) { - return callback(error) - } // check if request took too long and bail out. only do this for // get, because it is the first call in each update, so if this // passes we'll assume others have a reasonable chance to succeed. @@ -241,9 +218,9 @@ module.exports = RedisManager = { if (logHashReadErrors && computedHash !== storedHash) { logger.error( { - project_id, - doc_id, - doc_project_id, + projectId, + docId, + docProjectId, computedHash, storedHash, docLines, @@ -262,11 +239,8 @@ module.exports = RedisManager = { version = parseInt(version || 0, 10) // check doc is in requested project - if (doc_project_id != null && doc_project_id !== project_id) { - logger.error( - { project_id, doc_id, doc_project_id }, - 'doc not in project' - ) + if (docProjectId != null && docProjectId !== projectId) { + logger.error({ projectId, docId, docProjectId }, 'doc not in project') return callback(new Errors.NotFoundError('document not found')) } @@ -295,127 +269,109 @@ module.exports = RedisManager = { }) }, - getDocVersion(doc_id, callback) { - if (callback == null) { - callback = function () {} - } - return rclient.mget( - keys.docVersion({ doc_id }), - keys.projectHistoryType({ doc_id }), - function (error, result) { - if (error != null) { + getDocVersion(docId, callback) { + rclient.mget( + keys.docVersion({ doc_id: docId }), + keys.projectHistoryType({ doc_id: docId }), + (error, result) => { + if (error) { return callback(error) } - let [version, projectHistoryType] = Array.from(result || []) + let [version, projectHistoryType] = result || [] version = parseInt(version, 10) - return callback(null, version, projectHistoryType) + callback(null, version, projectHistoryType) } ) }, - getDocLines(doc_id, callback) { - if (callback == null) { - callback = function () {} - } - return rclient.get(keys.docLines({ doc_id }), function (error, docLines) { - if (error != null) { + getDocLines(docId, callback) { + rclient.get(keys.docLines({ doc_id: docId }), (error, docLines) => { + if (error) { return callback(error) } - return callback(null, docLines) + callback(null, docLines) }) }, - getPreviousDocOps(doc_id, start, end, callback) { - if (callback == null) { - callback = function () {} - } + getPreviousDocOps(docId, start, end, callback) { const timer = new metrics.Timer('redis.get-prev-docops') - return rclient.llen(keys.docOps({ doc_id }), function (error, length) { - if (error != null) { + rclient.llen(keys.docOps({ doc_id: docId }), (error, length) => { + if (error) { return callback(error) } - return rclient.get( - keys.docVersion({ doc_id }), - function (error, version) { - if (error != null) { - return callback(error) - } - version = parseInt(version, 10) - const first_version_in_redis = version - length - - if (start < first_version_in_redis || end > version) { - error = new Errors.OpRangeNotAvailableError( - 'doc ops range is not loaded in redis' - ) - logger.debug( - { err: error, doc_id, length, version, start, end }, - 'doc ops range is not loaded in redis' - ) - return callback(error) - } - - start = start - first_version_in_redis - if (end > -1) { - end = end - first_version_in_redis - } - - if (isNaN(start) || isNaN(end)) { - error = new Error('inconsistent version or lengths') - logger.error( - { err: error, doc_id, length, version, start, end }, - 'inconsistent version or length' - ) - return callback(error) - } - - return rclient.lrange( - keys.docOps({ doc_id }), - start, - end, - function (error, jsonOps) { - let ops - if (error != null) { - return callback(error) - } - try { - ops = jsonOps.map(jsonOp => JSON.parse(jsonOp)) - } catch (e) { - return callback(e) - } - const timeSpan = timer.done() - if (timeSpan > MAX_REDIS_REQUEST_LENGTH) { - error = new Error('redis getPreviousDocOps exceeded timeout') - return callback(error) - } - return callback(null, ops) - } - ) - } - ) - }) - }, - - getHistoryType(doc_id, callback) { - if (callback == null) { - callback = function () {} - } - return rclient.get( - keys.projectHistoryType({ doc_id }), - function (error, projectHistoryType) { - if (error != null) { + rclient.get(keys.docVersion({ doc_id: docId }), (error, version) => { + if (error) { return callback(error) } - return callback(null, projectHistoryType) + version = parseInt(version, 10) + const firstVersionInRedis = version - length + + if (start < firstVersionInRedis || end > version) { + error = new Errors.OpRangeNotAvailableError( + 'doc ops range is not loaded in redis' + ) + logger.debug( + { err: error, docId, length, version, start, end }, + 'doc ops range is not loaded in redis' + ) + return callback(error) + } + + start = start - firstVersionInRedis + if (end > -1) { + end = end - firstVersionInRedis + } + + if (isNaN(start) || isNaN(end)) { + error = new Error('inconsistent version or lengths') + logger.error( + { err: error, docId, length, version, start, end }, + 'inconsistent version or length' + ) + return callback(error) + } + + rclient.lrange( + keys.docOps({ doc_id: docId }), + start, + end, + (error, jsonOps) => { + let ops + if (error) { + return callback(error) + } + try { + ops = jsonOps.map(jsonOp => JSON.parse(jsonOp)) + } catch (e) { + return callback(e) + } + const timeSpan = timer.done() + if (timeSpan > MAX_REDIS_REQUEST_LENGTH) { + error = new Error('redis getPreviousDocOps exceeded timeout') + return callback(error) + } + callback(null, ops) + } + ) + }) + }) + }, + + getHistoryType(docId, callback) { + rclient.get( + keys.projectHistoryType({ doc_id: docId }), + (error, projectHistoryType) => { + if (error) { + return callback(error) + } + callback(null, projectHistoryType) } ) }, - setHistoryType(doc_id, projectHistoryType, callback) { - if (callback == null) { - callback = function () {} - } - return rclient.set( - keys.projectHistoryType({ doc_id }), + setHistoryType(docId, projectHistoryType, callback) { + rclient.set( + keys.projectHistoryType({ doc_id: docId }), projectHistoryType, callback ) @@ -424,8 +380,8 @@ module.exports = RedisManager = { DOC_OPS_TTL: 60 * minutes, DOC_OPS_MAX_LENGTH: 100, updateDocument( - project_id, - doc_id, + projectId, + docId, docLines, newVersion, appliedOps, @@ -436,21 +392,18 @@ module.exports = RedisManager = { if (appliedOps == null) { appliedOps = [] } - if (callback == null) { - callback = function () {} - } - return RedisManager.getDocVersion( - doc_id, - function (error, currentVersion, projectHistoryType) { - if (error != null) { + RedisManager.getDocVersion( + docId, + (error, currentVersion, projectHistoryType) => { + if (error) { return callback(error) } if (currentVersion + appliedOps.length !== newVersion) { - error = new Error(`Version mismatch. '${doc_id}' is corrupted.`) + error = new Error(`Version mismatch. '${docId}' is corrupted.`) logger.error( { err: error, - doc_id, + docId, currentVersion, newVersion, opsLength: appliedOps.length, @@ -461,11 +414,11 @@ module.exports = RedisManager = { } const jsonOps = appliedOps.map(op => JSON.stringify(op)) - for (const op of Array.from(jsonOps)) { + for (const op of jsonOps) { if (op.indexOf('\u0000') !== -1) { error = new Error('null bytes found in jsonOps') // this check was added to catch memory corruption in JSON.stringify - logger.error({ err: error, doc_id, jsonOps }, error.message) + logger.error({ err: error, docId, jsonOps }, error.message) return callback(error) } } @@ -474,7 +427,7 @@ module.exports = RedisManager = { if (newDocLines.indexOf('\u0000') !== -1) { error = new Error('null bytes found in doc lines') // this check was added to catch memory corruption in JSON.stringify - logger.error({ err: error, doc_id, newDocLines }, error.message) + logger.error({ err: error, docId, newDocLines }, error.message) return callback(error) } // Do an optimised size check on the docLines using the serialised @@ -483,15 +436,15 @@ module.exports = RedisManager = { if (docIsTooLarge(sizeBound, docLines, Settings.max_doc_length)) { const err = new Error('blocking doc update: doc is too large') const docSize = newDocLines.length - logger.error({ project_id, doc_id, err, docSize }, err.message) + logger.error({ projectId, docId, err, docSize }, err.message) return callback(err) } const newHash = RedisManager._computeHash(newDocLines) - const opVersions = appliedOps.map(op => (op != null ? op.v : undefined)) + const opVersions = appliedOps.map(op => op?.v) logger.debug( { - doc_id, + docId, version: newVersion, hash: newHash, op_versions: opVersions, @@ -502,61 +455,65 @@ module.exports = RedisManager = { metrics.summary('redis.docLines', newDocLines.length, { status: 'update', }) - return RedisManager._serializeRanges(ranges, function (error, ranges) { - if (error != null) { - logger.error({ err: error, doc_id }, error.message) + RedisManager._serializeRanges(ranges, (error, ranges) => { + if (error) { + logger.error({ err: error, docId }, error.message) return callback(error) } - if (ranges != null && ranges.indexOf('\u0000') !== -1) { + if (ranges && ranges.indexOf('\u0000') !== -1) { error = new Error('null bytes found in ranges') // this check was added to catch memory corruption in JSON.stringify - logger.error({ err: error, doc_id, ranges }, error.message) + logger.error({ err: error, docId, ranges }, error.message) return callback(error) } const multi = rclient.multi() multi.mset({ - [keys.docLines({ doc_id })]: newDocLines, - [keys.docVersion({ doc_id })]: newVersion, - [keys.docHash({ doc_id })]: newHash, - [keys.ranges({ doc_id })]: ranges, - [keys.lastUpdatedAt({ doc_id })]: Date.now(), - [keys.lastUpdatedBy({ doc_id })]: updateMeta && updateMeta.user_id, + [keys.docLines({ doc_id: docId })]: newDocLines, + [keys.docVersion({ doc_id: docId })]: newVersion, + [keys.docHash({ doc_id: docId })]: newHash, + [keys.ranges({ doc_id: docId })]: ranges, + [keys.lastUpdatedAt({ doc_id: docId })]: Date.now(), + [keys.lastUpdatedBy({ doc_id: docId })]: + updateMeta && updateMeta.user_id, }) multi.ltrim( - keys.docOps({ doc_id }), + keys.docOps({ doc_id: docId }), -RedisManager.DOC_OPS_MAX_LENGTH, -1 ) // index 3 // push the ops last so we can get the lengths at fixed index position 7 if (jsonOps.length > 0) { - multi.rpush(keys.docOps({ doc_id }), ...Array.from(jsonOps)) // index 5 + multi.rpush(keys.docOps({ doc_id: docId }), ...jsonOps) // index 5 // expire must come after rpush since before it will be a no-op if the list is empty - multi.expire(keys.docOps({ doc_id }), RedisManager.DOC_OPS_TTL) // index 6 + multi.expire( + keys.docOps({ doc_id: docId }), + RedisManager.DOC_OPS_TTL + ) // index 6 if (projectHistoryType === 'project-history') { metrics.inc('history-queue', 1, { status: 'skip-track-changes' }) logger.debug( - { doc_id }, + { docId }, 'skipping push of uncompressed ops for project using project-history' ) } else { // project is using old track-changes history service metrics.inc('history-queue', 1, { status: 'track-changes' }) multi.rpush( - historyKeys.uncompressedHistoryOps({ doc_id }), - ...Array.from(jsonOps) + historyKeys.uncompressedHistoryOps({ doc_id: docId }), + ...jsonOps ) // index 7 } // Set the unflushed timestamp to the current time if the doc // hasn't been modified before (the content in mongo has been // valid up to this point). Otherwise leave it alone ("NX" flag). - multi.set(keys.unflushedTime({ doc_id }), Date.now(), 'NX') + multi.set(keys.unflushedTime({ doc_id: docId }), Date.now(), 'NX') } - return multi.exec(function (error, result) { - let docUpdateCount - if (error != null) { + multi.exec((error, result) => { + if (error) { return callback(error) } + let docUpdateCount if (projectHistoryType === 'project-history') { docUpdateCount = undefined // only using project history, don't bother with track-changes } else { @@ -564,19 +521,11 @@ module.exports = RedisManager = { docUpdateCount = result[4] } - if ( - jsonOps.length > 0 && - __guard__( - Settings.apis != null - ? Settings.apis.project_history - : undefined, - x => x.enabled - ) - ) { + if (jsonOps.length > 0 && Settings.apis?.project_history?.enabled) { metrics.inc('history-queue', 1, { status: 'project-history' }) - return ProjectHistoryRedisManager.queueOps( - project_id, - ...Array.from(jsonOps), + ProjectHistoryRedisManager.queueOps( + projectId, + ...jsonOps, (error, projectUpdateCount) => { if (error) { // The full project history can re-sync a project in case @@ -588,7 +537,7 @@ module.exports = RedisManager = { } ) } else { - return callback(null, docUpdateCount) + callback(null, docUpdateCount) } }) }) @@ -596,86 +545,74 @@ module.exports = RedisManager = { ) }, - renameDoc(project_id, doc_id, user_id, update, projectHistoryId, callback) { - if (callback == null) { - callback = function () {} - } - return RedisManager.getDoc( - project_id, - doc_id, - function (error, lines, version) { - if (error != null) { - return callback(error) - } - if (lines != null && version != null) { - if (!update.newPathname) { - logger.warn( - { project_id, doc_id, update }, - 'missing pathname in RedisManager.renameDoc' - ) - metrics.inc('pathname', 1, { - path: 'RedisManager.renameDoc', - status: update.newPathname === '' ? 'zero-length' : 'undefined', - }) - } - return rclient.set( - keys.pathname({ doc_id }), - update.newPathname, - callback - ) - } else { - return callback() - } + renameDoc(projectId, docId, userId, update, projectHistoryId, callback) { + RedisManager.getDoc(projectId, docId, (error, lines, version) => { + if (error) { + return callback(error) } - ) + if (lines != null && version != null) { + if (!update.newPathname) { + logger.warn( + { projectId, docId, update }, + 'missing pathname in RedisManager.renameDoc' + ) + metrics.inc('pathname', 1, { + path: 'RedisManager.renameDoc', + status: update.newPathname === '' ? 'zero-length' : 'undefined', + }) + } + rclient.set( + keys.pathname({ doc_id: docId }), + update.newPathname, + callback + ) + } else { + callback() + } + }) }, - clearUnflushedTime(doc_id, callback) { - if (callback == null) { - callback = function () {} - } - return rclient.del(keys.unflushedTime({ doc_id }), callback) + clearUnflushedTime(docId, callback) { + rclient.del(keys.unflushedTime({ doc_id: docId }), callback) }, - getDocIdsInProject(project_id, callback) { - if (callback == null) { - callback = function () {} - } - return rclient.smembers(keys.docsInProject({ project_id }), callback) + getDocIdsInProject(projectId, callback) { + rclient.smembers(keys.docsInProject({ project_id: projectId }), callback) }, - getDocTimestamps(doc_ids, callback) { - // get lastupdatedat timestamps for an array of doc_ids - if (callback == null) { - callback = function () {} - } - return async.mapSeries( - doc_ids, - (doc_id, cb) => rclient.get(keys.lastUpdatedAt({ doc_id }), cb), + /** + * Get lastupdatedat timestamps for an array of docIds + */ + getDocTimestamps(docIds, callback) { + async.mapSeries( + docIds, + (docId, cb) => rclient.get(keys.lastUpdatedAt({ doc_id: docId }), cb), callback ) }, - queueFlushAndDeleteProject(project_id, callback) { - // store the project id in a sorted set ordered by time with a random offset to smooth out spikes + /** + * Store the project id in a sorted set ordered by time with a random offset + * to smooth out spikes + */ + queueFlushAndDeleteProject(projectId, callback) { const SMOOTHING_OFFSET = Settings.smoothingOffset > 0 ? Math.round(Settings.smoothingOffset * Math.random()) : 0 - return rclient.zadd( + rclient.zadd( keys.flushAndDeleteQueue(), Date.now() + SMOOTHING_OFFSET, - project_id, + projectId, callback ) }, + /** + * Find the oldest queued flush that is before the cutoff time + */ getNextProjectToFlushAndDelete(cutoffTime, callback) { - // find the oldest queued flush that is before the cutoff time - if (callback == null) { - callback = function () {} - } - return rclient.zrangebyscore( + rclient.zrangebyscore( keys.flushAndDeleteQueue(), 0, cutoffTime, @@ -683,47 +620,45 @@ module.exports = RedisManager = { 'LIMIT', 0, 1, - function (err, reply) { - if (err != null) { + (err, reply) => { + if (err) { return callback(err) } - if (!(reply != null ? reply.length : undefined)) { + // return if no projects ready to be processed + if (!reply || reply.length === 0) { return callback() - } // return if no projects ready to be processed + } // pop the oldest entry (get and remove in a multi) const multi = rclient.multi() // Poor man's version of ZPOPMIN, which is only available in Redis 5. multi.zrange(keys.flushAndDeleteQueue(), 0, 0, 'WITHSCORES') multi.zremrangebyrank(keys.flushAndDeleteQueue(), 0, 0) multi.zcard(keys.flushAndDeleteQueue()) // the total length of the queue (for metrics) - return multi.exec(function (err, reply) { - if (err != null) { + multi.exec((err, reply) => { + if (err) { return callback(err) } - if (!(reply != null ? reply.length : undefined)) { + if (!reply || reply.length === 0) { return callback() } - const [key, timestamp] = Array.from(reply[0]) + const [key, timestamp] = reply[0] const queueLength = reply[2] - return callback(null, key, timestamp, queueLength) + callback(null, key, timestamp, queueLength) }) } ) }, _serializeRanges(ranges, callback) { - if (callback == null) { - callback = function () {} - } let jsonRanges = JSON.stringify(ranges) - if (jsonRanges != null && jsonRanges.length > MAX_RANGES_SIZE) { + if (jsonRanges && jsonRanges.length > MAX_RANGES_SIZE) { return callback(new Error('ranges are too large')) } if (jsonRanges === '{}') { // Most doc will have empty ranges so don't fill redis with lots of '{}' keys jsonRanges = null } - return callback(null, jsonRanges) + callback(null, jsonRanges) }, _deserializeRanges(ranges) { @@ -742,9 +677,3 @@ module.exports = RedisManager = { return crypto.createHash('sha1').update(docLines, 'utf8').digest('hex') }, } - -function __guard__(value, transform) { - return typeof value !== 'undefined' && value !== null - ? transform(value) - : undefined -} diff --git a/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js b/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js index cedeb9a34f..d983ab01cd 100644 --- a/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js +++ b/services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js @@ -1,16 +1,3 @@ -/* eslint-disable - camelcase, - mocha/no-identical-title, - 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 modulePath = '../../../../app/js/RedisManager.js' const SandboxedModule = require('sandboxed-module') @@ -20,7 +7,6 @@ const tk = require('timekeeper') describe('RedisManager', function () { beforeEach(function () { - let Timer this.multi = { exec: sinon.stub() } this.rclient = { multi: () => this.multi } tk.freeze(new Date()) @@ -35,63 +21,63 @@ describe('RedisManager', function () { redis: { documentupdater: { key_schema: { - blockingKey({ doc_id }) { - return `Blocking:${doc_id}` + blockingKey({ doc_id: docId }) { + return `Blocking:${docId}` }, - docLines({ doc_id }) { - return `doclines:${doc_id}` + docLines({ doc_id: docId }) { + return `doclines:${docId}` }, - docOps({ doc_id }) { - return `DocOps:${doc_id}` + docOps({ doc_id: docId }) { + return `DocOps:${docId}` }, - docVersion({ doc_id }) { - return `DocVersion:${doc_id}` + docVersion({ doc_id: docId }) { + return `DocVersion:${docId}` }, - docHash({ doc_id }) { - return `DocHash:${doc_id}` + docHash({ doc_id: docId }) { + return `DocHash:${docId}` }, - projectKey({ doc_id }) { - return `ProjectId:${doc_id}` + projectKey({ doc_id: docId }) { + return `ProjectId:${docId}` }, - pendingUpdates({ doc_id }) { - return `PendingUpdates:${doc_id}` + pendingUpdates({ doc_id: docId }) { + return `PendingUpdates:${docId}` }, - docsInProject({ project_id }) { - return `DocsIn:${project_id}` + docsInProject({ project_id: projectId }) { + return `DocsIn:${projectId}` }, - ranges({ doc_id }) { - return `Ranges:${doc_id}` + ranges({ doc_id: docId }) { + return `Ranges:${docId}` }, - pathname({ doc_id }) { - return `Pathname:${doc_id}` + pathname({ doc_id: docId }) { + return `Pathname:${docId}` }, - projectHistoryId({ doc_id }) { - return `ProjectHistoryId:${doc_id}` + projectHistoryId({ doc_id: docId }) { + return `ProjectHistoryId:${docId}` }, - projectHistoryType({ doc_id }) { - return `ProjectHistoryType:${doc_id}` + projectHistoryType({ doc_id: docId }) { + return `ProjectHistoryType:${docId}` }, - projectState({ project_id }) { - return `ProjectState:${project_id}` + projectState({ project_id: projectId }) { + return `ProjectState:${projectId}` }, - unflushedTime({ doc_id }) { - return `UnflushedTime:${doc_id}` + unflushedTime({ doc_id: docId }) { + return `UnflushedTime:${docId}` }, - lastUpdatedBy({ doc_id }) { - return `lastUpdatedBy:${doc_id}` + lastUpdatedBy({ doc_id: docId }) { + return `lastUpdatedBy:${docId}` }, - lastUpdatedAt({ doc_id }) { - return `lastUpdatedAt:${doc_id}` + lastUpdatedAt({ doc_id: docId }) { + return `lastUpdatedAt:${docId}` }, }, }, history: { key_schema: { - uncompressedHistoryOps({ doc_id }) { - return `UncompressedHistoryOps:${doc_id}` + uncompressedHistoryOps({ doc_id: docId }) { + return `UncompressedHistoryOps:${docId}` }, - docsWithHistoryOps({ project_id }) { - return `DocsWithHistoryOps:${project_id}` + docsWithHistoryOps({ project_id: projectId }) { + return `DocsWithHistoryOps:${projectId}` }, }, }, @@ -103,7 +89,7 @@ describe('RedisManager', function () { './Metrics': (this.metrics = { inc: sinon.stub(), summary: sinon.stub(), - Timer: (Timer = class Timer { + Timer: class Timer { constructor() { this.start = new Date() } @@ -112,20 +98,20 @@ describe('RedisManager', function () { const timeSpan = new Date() - this.start return timeSpan } - }), + }, }), './Errors': Errors, }, }) - this.doc_id = 'doc-id-123' + this.docId = 'doc-id-123' this.project_id = 'project-id-123' this.projectHistoryId = 123 - return (this.callback = sinon.stub()) + this.callback = sinon.stub() }) afterEach(function () { - return tk.reset() + tk.reset() }) describe('getDoc', function () { @@ -157,32 +143,28 @@ describe('RedisManager', function () { describe('successfully', function () { beforeEach(function () { - return this.RedisManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) + this.RedisManager.getDoc(this.project_id, this.docId, this.callback) }) it('should get all the details in one call to redis', function () { this.rclient.mget .calledWith( - `doclines:${this.doc_id}`, - `DocVersion:${this.doc_id}`, - `DocHash:${this.doc_id}`, - `ProjectId:${this.doc_id}`, - `Ranges:${this.doc_id}`, - `Pathname:${this.doc_id}`, - `ProjectHistoryId:${this.doc_id}`, - `UnflushedTime:${this.doc_id}`, - `lastUpdatedAt:${this.doc_id}`, - `lastUpdatedBy:${this.doc_id}` + `doclines:${this.docId}`, + `DocVersion:${this.docId}`, + `DocHash:${this.docId}`, + `ProjectId:${this.docId}`, + `Ranges:${this.docId}`, + `Pathname:${this.docId}`, + `ProjectHistoryId:${this.docId}`, + `UnflushedTime:${this.docId}`, + `lastUpdatedAt:${this.docId}`, + `lastUpdatedBy:${this.docId}` ) .should.equal(true) }) it('should return the document', function () { - return this.callback + this.callback .calledWithExactly( null, this.lines, @@ -197,8 +179,8 @@ describe('RedisManager', function () { .should.equal(true) }) - return it('should not log any errors', function () { - return this.logger.error.calledWith().should.equal(false) + it('should not log any errors', function () { + this.logger.error.calledWith().should.equal(false) }) }) @@ -214,19 +196,15 @@ describe('RedisManager', function () { this.project_id, this.json_ranges, ]) - return this.RedisManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) + this.RedisManager.getDoc(this.project_id, this.docId, this.callback) }) it('should log a hash error', function () { - return this.logger.error.calledWith().should.equal(true) + this.logger.error.calledWith().should.equal(true) }) - return it('should return the document', function () { - return this.callback + it('should return the document', function () { + this.callback .calledWith(null, this.lines, this.version, this.ranges) .should.equal(true) }) @@ -238,7 +216,7 @@ describe('RedisManager', function () { this.rclient.mget = (...args) => { const cb = args.pop() this.clock.tick(6000) - return cb(null, [ + cb(null, [ this.jsonlines, this.version, this.another_project_id, @@ -248,25 +226,21 @@ describe('RedisManager', function () { ]) } - return this.RedisManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) + this.RedisManager.getDoc(this.project_id, this.docId, this.callback) }) afterEach(function () { - return this.clock.restore() + this.clock.restore() }) - return it('should return an error', function () { - return this.callback + it('should return an error', function () { + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) }) - return describe('getDoc with an invalid project id', function () { + describe('getDoc with an invalid project id', function () { beforeEach(function () { this.another_project_id = 'project-id-456' this.rclient.mget = sinon @@ -280,15 +254,11 @@ describe('RedisManager', function () { this.pathname, this.unflushed_time, ]) - return this.RedisManager.getDoc( - this.project_id, - this.doc_id, - this.callback - ) + this.RedisManager.getDoc(this.project_id, this.docId, this.callback) }) - return it('should return an error', function () { - return this.callback + it('should return an error', function () { + this.callback .calledWith(sinon.match.instanceOf(Errors.NotFoundError)) .should.equal(true) }) @@ -310,8 +280,8 @@ describe('RedisManager', function () { .stub() .callsArgWith(1, null, this.version.toString()) this.rclient.lrange = sinon.stub().callsArgWith(3, null, this.jsonOps) - return this.RedisManager.getPreviousDocOps( - this.doc_id, + this.RedisManager.getPreviousDocOps( + this.docId, this.start, this.end, this.callback @@ -319,29 +289,27 @@ describe('RedisManager', function () { }) it('should get the length of the existing doc ops', function () { - return this.rclient.llen - .calledWith(`DocOps:${this.doc_id}`) - .should.equal(true) + this.rclient.llen.calledWith(`DocOps:${this.docId}`).should.equal(true) }) it('should get the current version of the doc', function () { - return this.rclient.get - .calledWith(`DocVersion:${this.doc_id}`) + this.rclient.get + .calledWith(`DocVersion:${this.docId}`) .should.equal(true) }) it('should get the appropriate docs ops', function () { - return this.rclient.lrange + this.rclient.lrange .calledWith( - `DocOps:${this.doc_id}`, + `DocOps:${this.docId}`, this.start - this.first_version_in_redis, this.end - this.first_version_in_redis ) .should.equal(true) }) - return it('should return the docs with the doc ops deserialized', function () { - return this.callback.calledWith(null, this.ops).should.equal(true) + it('should return the docs with the doc ops deserialized', function () { + this.callback.calledWith(null, this.ops).should.equal(true) }) }) @@ -359,8 +327,8 @@ describe('RedisManager', function () { .stub() .callsArgWith(1, null, this.version.toString()) this.rclient.lrange = sinon.stub().callsArgWith(3, null, this.jsonOps) - return this.RedisManager.getPreviousDocOps( - this.doc_id, + this.RedisManager.getPreviousDocOps( + this.docId, this.start, this.end, this.callback @@ -368,17 +336,17 @@ describe('RedisManager', function () { }) it('should get the appropriate docs ops to the end of list', function () { - return this.rclient.lrange + this.rclient.lrange .calledWith( - `DocOps:${this.doc_id}`, + `DocOps:${this.docId}`, this.start - this.first_version_in_redis, -1 ) .should.equal(true) }) - return it('should return the docs with the doc ops deserialized', function () { - return this.callback.calledWith(null, this.ops).should.equal(true) + it('should return the docs with the doc ops deserialized', function () { + this.callback.calledWith(null, this.ops).should.equal(true) }) }) @@ -396,8 +364,8 @@ describe('RedisManager', function () { .stub() .callsArgWith(1, null, this.version.toString()) this.rclient.lrange = sinon.stub().callsArgWith(3, null, this.jsonOps) - return this.RedisManager.getPreviousDocOps( - this.doc_id, + this.RedisManager.getPreviousDocOps( + this.docId, this.start, this.end, this.callback @@ -405,17 +373,17 @@ describe('RedisManager', function () { }) it('should return an error', function () { - return this.callback + this.callback .calledWith(sinon.match.instanceOf(Errors.OpRangeNotAvailableError)) .should.equal(true) }) - return it('should log out the problem as a debug message', function () { - return this.logger.debug.called.should.equal(true) + it('should log out the problem as a debug message', function () { + this.logger.debug.called.should.equal(true) }) }) - return describe('with a slow request to redis', function () { + describe('with a slow request to redis', function () { beforeEach(function () { this.first_version_in_redis = 30 this.version = 70 @@ -431,10 +399,10 @@ describe('RedisManager', function () { this.clock = sinon.useFakeTimers() this.rclient.lrange = (key, start, end, cb) => { this.clock.tick(6000) - return cb(null, this.jsonOps) + cb(null, this.jsonOps) } - return this.RedisManager.getPreviousDocOps( - this.doc_id, + this.RedisManager.getPreviousDocOps( + this.docId, this.start, this.end, this.callback @@ -442,11 +410,11 @@ describe('RedisManager', function () { }) afterEach(function () { - return this.clock.restore() + this.clock.restore() }) - return it('should return an error', function () { - return this.callback + it('should return an error', function () { + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) @@ -485,13 +453,13 @@ describe('RedisManager', function () { null, null, ]) - return (this.ProjectHistoryRedisManager.queueOps = sinon + this.ProjectHistoryRedisManager.queueOps = sinon .stub() .callsArgWith( this.ops.length + 1, null, this.project_update_list_length - )) + ) }) describe('with a consistent version', function () { @@ -501,11 +469,11 @@ describe('RedisManager', function () { beforeEach(function () { this.settings.apis.project_history.enabled = true this.RedisManager.getDocVersion - .withArgs(this.doc_id) + .withArgs(this.docId) .yields(null, this.version - this.ops.length) - return this.RedisManager.updateDocument( + this.RedisManager.updateDocument( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ops, @@ -516,34 +484,34 @@ describe('RedisManager', function () { }) it('should get the current doc version to check for consistency', function () { - return this.RedisManager.getDocVersion - .calledWith(this.doc_id) + this.RedisManager.getDocVersion + .calledWith(this.docId) .should.equal(true) }) it('should set most details in a single MSET call', function () { this.multi.mset .calledWith({ - [`doclines:${this.doc_id}`]: JSON.stringify(this.lines), - [`DocVersion:${this.doc_id}`]: this.version, - [`DocHash:${this.doc_id}`]: this.hash, - [`Ranges:${this.doc_id}`]: JSON.stringify(this.ranges), - [`lastUpdatedAt:${this.doc_id}`]: Date.now(), - [`lastUpdatedBy:${this.doc_id}`]: 'last-author-fake-id', + [`doclines:${this.docId}`]: JSON.stringify(this.lines), + [`DocVersion:${this.docId}`]: this.version, + [`DocHash:${this.docId}`]: this.hash, + [`Ranges:${this.docId}`]: JSON.stringify(this.ranges), + [`lastUpdatedAt:${this.docId}`]: Date.now(), + [`lastUpdatedBy:${this.docId}`]: 'last-author-fake-id', }) .should.equal(true) }) it('should set the unflushed time', function () { - return this.multi.set - .calledWith(`UnflushedTime:${this.doc_id}`, Date.now(), 'NX') + this.multi.set + .calledWith(`UnflushedTime:${this.docId}`, Date.now(), 'NX') .should.equal(true) }) it('should push the doc op into the doc ops list', function () { - return this.multi.rpush + this.multi.rpush .calledWith( - `DocOps:${this.doc_id}`, + `DocOps:${this.docId}`, JSON.stringify(this.ops[0]), JSON.stringify(this.ops[1]) ) @@ -551,15 +519,15 @@ describe('RedisManager', function () { }) it('should renew the expiry ttl on the doc ops array', function () { - return this.multi.expire - .calledWith(`DocOps:${this.doc_id}`, this.RedisManager.DOC_OPS_TTL) + this.multi.expire + .calledWith(`DocOps:${this.docId}`, this.RedisManager.DOC_OPS_TTL) .should.equal(true) }) it('should truncate the list to 100 members', function () { - return this.multi.ltrim + this.multi.ltrim .calledWith( - `DocOps:${this.doc_id}`, + `DocOps:${this.docId}`, -this.RedisManager.DOC_OPS_MAX_LENGTH, -1 ) @@ -567,9 +535,9 @@ describe('RedisManager', function () { }) it('should push the updates into the history ops list', function () { - return this.multi.rpush + this.multi.rpush .calledWith( - `UncompressedHistoryOps:${this.doc_id}`, + `UncompressedHistoryOps:${this.docId}`, JSON.stringify(this.ops[0]), JSON.stringify(this.ops[1]) ) @@ -577,13 +545,13 @@ describe('RedisManager', function () { }) it('should push the updates into the project history ops list', function () { - return this.ProjectHistoryRedisManager.queueOps + this.ProjectHistoryRedisManager.queueOps .calledWith(this.project_id, JSON.stringify(this.ops[0])) .should.equal(true) }) it('should call the callback', function () { - return this.callback + this.callback .calledWith( null, this.doc_update_list_length, @@ -592,8 +560,8 @@ describe('RedisManager', function () { .should.equal(true) }) - return it('should not log any errors', function () { - return this.logger.error.calledWith().should.equal(false) + it('should not log any errors', function () { + this.logger.error.calledWith().should.equal(false) }) }) @@ -601,11 +569,11 @@ describe('RedisManager', function () { beforeEach(function () { this.settings.apis.project_history.enabled = false this.RedisManager.getDocVersion - .withArgs(this.doc_id) + .withArgs(this.docId) .yields(null, this.version - this.ops.length) - return this.RedisManager.updateDocument( + this.RedisManager.updateDocument( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ops, @@ -616,26 +584,24 @@ describe('RedisManager', function () { }) it('should not push the updates into the project history ops list', function () { - return this.ProjectHistoryRedisManager.queueOps.called.should.equal( - false - ) + this.ProjectHistoryRedisManager.queueOps.called.should.equal(false) }) - return it('should call the callback', function () { - return this.callback + it('should call the callback', function () { + this.callback .calledWith(null, this.doc_update_list_length) .should.equal(true) }) }) - return describe('with a doc using project history only', function () { + describe('with a doc using project history only', function () { beforeEach(function () { this.RedisManager.getDocVersion - .withArgs(this.doc_id) + .withArgs(this.docId) .yields(null, this.version - this.ops.length, 'project-history') - return this.RedisManager.updateDocument( + this.RedisManager.updateDocument( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ops, @@ -646,19 +612,19 @@ describe('RedisManager', function () { }) it('should not push the updates to the track-changes ops list', function () { - return this.multi.rpush - .calledWith(`UncompressedHistoryOps:${this.doc_id}`) + this.multi.rpush + .calledWith(`UncompressedHistoryOps:${this.docId}`) .should.equal(false) }) it('should push the updates into the project history ops list', function () { - return this.ProjectHistoryRedisManager.queueOps + this.ProjectHistoryRedisManager.queueOps .calledWith(this.project_id, JSON.stringify(this.ops[0])) .should.equal(true) }) - return it('should call the callback with the project update count only', function () { - return this.callback + it('should call the callback with the project update count only', function () { + this.callback .calledWith(null, undefined, this.project_update_list_length) .should.equal(true) }) @@ -668,11 +634,11 @@ describe('RedisManager', function () { describe('with an inconsistent version', function () { beforeEach(function () { this.RedisManager.getDocVersion - .withArgs(this.doc_id) + .withArgs(this.docId) .yields(null, this.version - this.ops.length - 1) - return this.RedisManager.updateDocument( + this.RedisManager.updateDocument( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ops, @@ -683,11 +649,11 @@ describe('RedisManager', function () { }) it('should not call multi.exec', function () { - return this.multi.exec.called.should.equal(false) + this.multi.exec.called.should.equal(false) }) - return it('should call the callback with an error', function () { - return this.callback + it('should call the callback with an error', function () { + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) @@ -696,11 +662,11 @@ describe('RedisManager', function () { describe('with no updates', function () { beforeEach(function () { this.RedisManager.getDocVersion - .withArgs(this.doc_id) + .withArgs(this.docId) .yields(null, this.version) - return this.RedisManager.updateDocument( + this.RedisManager.updateDocument( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, [], @@ -711,24 +677,22 @@ describe('RedisManager', function () { }) it('should not try to enqueue doc updates', function () { - return this.multi.rpush.called.should.equal(false) + this.multi.rpush.called.should.equal(false) }) it('should not try to enqueue project updates', function () { - return this.ProjectHistoryRedisManager.queueOps.called.should.equal( - false - ) + this.ProjectHistoryRedisManager.queueOps.called.should.equal(false) }) - return it('should still set the doclines', function () { + it('should still set the doclines', function () { this.multi.mset .calledWith({ - [`doclines:${this.doc_id}`]: JSON.stringify(this.lines), - [`DocVersion:${this.doc_id}`]: this.version, - [`DocHash:${this.doc_id}`]: this.hash, - [`Ranges:${this.doc_id}`]: JSON.stringify(this.ranges), - [`lastUpdatedAt:${this.doc_id}`]: Date.now(), - [`lastUpdatedBy:${this.doc_id}`]: 'last-author-fake-id', + [`doclines:${this.docId}`]: JSON.stringify(this.lines), + [`DocVersion:${this.docId}`]: this.version, + [`DocHash:${this.docId}`]: this.hash, + [`Ranges:${this.docId}`]: JSON.stringify(this.ranges), + [`lastUpdatedAt:${this.docId}`]: Date.now(), + [`lastUpdatedBy:${this.docId}`]: 'last-author-fake-id', }) .should.equal(true) }) @@ -737,11 +701,11 @@ describe('RedisManager', function () { describe('with empty ranges', function () { beforeEach(function () { this.RedisManager.getDocVersion - .withArgs(this.doc_id) + .withArgs(this.docId) .yields(null, this.version - this.ops.length) - return this.RedisManager.updateDocument( + this.RedisManager.updateDocument( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ops, @@ -754,12 +718,12 @@ describe('RedisManager', function () { it('should set empty ranges', function () { this.multi.mset .calledWith({ - [`doclines:${this.doc_id}`]: JSON.stringify(this.lines), - [`DocVersion:${this.doc_id}`]: this.version, - [`DocHash:${this.doc_id}`]: this.hash, - [`Ranges:${this.doc_id}`]: null, - [`lastUpdatedAt:${this.doc_id}`]: Date.now(), - [`lastUpdatedBy:${this.doc_id}`]: 'last-author-fake-id', + [`doclines:${this.docId}`]: JSON.stringify(this.lines), + [`DocVersion:${this.docId}`]: this.version, + [`DocHash:${this.docId}`]: this.hash, + [`Ranges:${this.docId}`]: null, + [`lastUpdatedAt:${this.docId}`]: Date.now(), + [`lastUpdatedBy:${this.docId}`]: 'last-author-fake-id', }) .should.equal(true) }) @@ -768,14 +732,14 @@ describe('RedisManager', function () { describe('with null bytes in the serialized doc lines', function () { beforeEach(function () { this.RedisManager.getDocVersion - .withArgs(this.doc_id) + .withArgs(this.docId) .yields(null, this.version - this.ops.length) this.stringifyStub = sinon .stub(JSON, 'stringify') .callsFake(() => '["bad bytes! \u0000 <- here"]') - return this.RedisManager.updateDocument( + this.RedisManager.updateDocument( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ops, @@ -790,11 +754,11 @@ describe('RedisManager', function () { }) it('should log an error', function () { - return this.logger.error.called.should.equal(true) + this.logger.error.called.should.equal(true) }) - return it('should call the callback with an error', function () { - return this.callback + it('should call the callback with an error', function () { + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) @@ -803,14 +767,14 @@ describe('RedisManager', function () { describe('with ranges that are too big', function () { beforeEach(function () { this.RedisManager.getDocVersion - .withArgs(this.doc_id) + .withArgs(this.docId) .yields(null, this.version - this.ops.length) this.RedisManager._serializeRanges = sinon .stub() .yields(new Error('ranges are too large')) - return this.RedisManager.updateDocument( + this.RedisManager.updateDocument( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ops, @@ -821,24 +785,24 @@ describe('RedisManager', function () { }) it('should log an error', function () { - return this.logger.error.called.should.equal(true) + this.logger.error.called.should.equal(true) }) - return it('should call the callback with the error', function () { - return this.callback + it('should call the callback with the error', function () { + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) }) - return describe('without user id from meta', function () { + describe('without user id from meta', function () { beforeEach(function () { this.RedisManager.getDocVersion - .withArgs(this.doc_id) + .withArgs(this.docId) .yields(null, this.version - this.ops.length) - return this.RedisManager.updateDocument( + this.RedisManager.updateDocument( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ops, @@ -851,12 +815,12 @@ describe('RedisManager', function () { it('should unset last updater', function () { this.multi.mset .calledWith({ - [`doclines:${this.doc_id}`]: JSON.stringify(this.lines), - [`DocVersion:${this.doc_id}`]: this.version, - [`DocHash:${this.doc_id}`]: this.hash, - [`Ranges:${this.doc_id}`]: JSON.stringify(this.ranges), - [`lastUpdatedAt:${this.doc_id}`]: Date.now(), - [`lastUpdatedBy:${this.doc_id}`]: undefined, + [`doclines:${this.docId}`]: JSON.stringify(this.lines), + [`DocVersion:${this.docId}`]: this.version, + [`DocHash:${this.docId}`]: this.hash, + [`Ranges:${this.docId}`]: JSON.stringify(this.ranges), + [`lastUpdatedAt:${this.docId}`]: Date.now(), + [`lastUpdatedBy:${this.docId}`]: undefined, }) .should.equal(true) }) @@ -874,14 +838,14 @@ describe('RedisManager', function () { .update(JSON.stringify(this.lines), 'utf8') .digest('hex') this.ranges = { comments: 'mock', entries: 'mock' } - return (this.pathname = '/a/b/c.tex') + this.pathname = '/a/b/c.tex' }) describe('with non-empty ranges', function () { beforeEach(function (done) { - return this.RedisManager.putDocInMemory( + this.RedisManager.putDocInMemory( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ranges, @@ -894,33 +858,33 @@ describe('RedisManager', function () { it('should set all the details in a single MSET call', function () { this.rclient.mset .calledWith({ - [`doclines:${this.doc_id}`]: JSON.stringify(this.lines), - [`ProjectId:${this.doc_id}`]: this.project_id, - [`DocVersion:${this.doc_id}`]: this.version, - [`DocHash:${this.doc_id}`]: this.hash, - [`Ranges:${this.doc_id}`]: JSON.stringify(this.ranges), - [`Pathname:${this.doc_id}`]: this.pathname, - [`ProjectHistoryId:${this.doc_id}`]: this.projectHistoryId, + [`doclines:${this.docId}`]: JSON.stringify(this.lines), + [`ProjectId:${this.docId}`]: this.project_id, + [`DocVersion:${this.docId}`]: this.version, + [`DocHash:${this.docId}`]: this.hash, + [`Ranges:${this.docId}`]: JSON.stringify(this.ranges), + [`Pathname:${this.docId}`]: this.pathname, + [`ProjectHistoryId:${this.docId}`]: this.projectHistoryId, }) .should.equal(true) }) - it('should add the doc_id to the project set', function () { - return this.rclient.sadd - .calledWith(`DocsIn:${this.project_id}`, this.doc_id) + it('should add the docId to the project set', function () { + this.rclient.sadd + .calledWith(`DocsIn:${this.project_id}`, this.docId) .should.equal(true) }) - return it('should not log any errors', function () { - return this.logger.error.calledWith().should.equal(false) + it('should not log any errors', function () { + this.logger.error.calledWith().should.equal(false) }) }) describe('with empty ranges', function () { beforeEach(function (done) { - return this.RedisManager.putDocInMemory( + this.RedisManager.putDocInMemory( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, {}, @@ -933,13 +897,13 @@ describe('RedisManager', function () { it('should unset ranges', function () { this.rclient.mset .calledWith({ - [`doclines:${this.doc_id}`]: JSON.stringify(this.lines), - [`ProjectId:${this.doc_id}`]: this.project_id, - [`DocVersion:${this.doc_id}`]: this.version, - [`DocHash:${this.doc_id}`]: this.hash, - [`Ranges:${this.doc_id}`]: null, - [`Pathname:${this.doc_id}`]: this.pathname, - [`ProjectHistoryId:${this.doc_id}`]: this.projectHistoryId, + [`doclines:${this.docId}`]: JSON.stringify(this.lines), + [`ProjectId:${this.docId}`]: this.project_id, + [`DocVersion:${this.docId}`]: this.version, + [`DocHash:${this.docId}`]: this.hash, + [`Ranges:${this.docId}`]: null, + [`Pathname:${this.docId}`]: this.pathname, + [`ProjectHistoryId:${this.docId}`]: this.projectHistoryId, }) .should.equal(true) }) @@ -950,9 +914,9 @@ describe('RedisManager', function () { this.stringifyStub = sinon .stub(JSON, 'stringify') .callsFake(() => '["bad bytes! \u0000 <- here"]') - return this.RedisManager.putDocInMemory( + this.RedisManager.putDocInMemory( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ranges, @@ -967,24 +931,24 @@ describe('RedisManager', function () { }) it('should log an error', function () { - return this.logger.error.called.should.equal(true) + this.logger.error.called.should.equal(true) }) - return it('should call the callback with an error', function () { - return this.callback + it('should call the callback with an error', function () { + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) }) - return describe('with ranges that are too big', function () { + describe('with ranges that are too big', function () { beforeEach(function () { this.RedisManager._serializeRanges = sinon .stub() .yields(new Error('ranges are too large')) - return this.RedisManager.putDocInMemory( + this.RedisManager.putDocInMemory( this.project_id, - this.doc_id, + this.docId, this.lines, this.version, this.ranges, @@ -995,11 +959,11 @@ describe('RedisManager', function () { }) it('should log an error', function () { - return this.logger.error.called.should.equal(true) + this.logger.error.called.should.equal(true) }) - return it('should call the callback with the error', function () { - return this.callback + it('should call the callback with the error', function () { + this.callback .calledWith(sinon.match.instanceOf(Error)) .should.equal(true) }) @@ -1012,40 +976,34 @@ describe('RedisManager', function () { this.multi.del = sinon.stub() this.multi.srem = sinon.stub() this.multi.exec.yields() - return this.RedisManager.removeDocFromMemory( - this.project_id, - this.doc_id, - done - ) + this.RedisManager.removeDocFromMemory(this.project_id, this.docId, done) }) it('should check the length of the current doclines', function () { - return this.multi.strlen - .calledWith(`doclines:${this.doc_id}`) - .should.equal(true) + this.multi.strlen.calledWith(`doclines:${this.docId}`).should.equal(true) }) it('should delete the details in a singe call', function () { - return this.multi.del + this.multi.del .calledWith( - `doclines:${this.doc_id}`, - `ProjectId:${this.doc_id}`, - `DocVersion:${this.doc_id}`, - `DocHash:${this.doc_id}`, - `Ranges:${this.doc_id}`, - `Pathname:${this.doc_id}`, - `ProjectHistoryId:${this.doc_id}`, - `ProjectHistoryType:${this.doc_id}`, - `UnflushedTime:${this.doc_id}`, - `lastUpdatedAt:${this.doc_id}`, - `lastUpdatedBy:${this.doc_id}` + `doclines:${this.docId}`, + `ProjectId:${this.docId}`, + `DocVersion:${this.docId}`, + `DocHash:${this.docId}`, + `Ranges:${this.docId}`, + `Pathname:${this.docId}`, + `ProjectHistoryId:${this.docId}`, + `ProjectHistoryType:${this.docId}`, + `UnflushedTime:${this.docId}`, + `lastUpdatedAt:${this.docId}`, + `lastUpdatedBy:${this.docId}` ) .should.equal(true) }) - it('should remove the doc_id from the project set', function () { - return this.multi.srem - .calledWith(`DocsIn:${this.project_id}`, this.doc_id) + it('should remove the docId from the project set', function () { + this.multi.srem + .calledWith(`DocsIn:${this.project_id}`, this.docId) .should.equal(true) }) }) @@ -1053,25 +1011,25 @@ describe('RedisManager', function () { describe('clearProjectState', function () { beforeEach(function (done) { this.rclient.del = sinon.stub().callsArg(1) - return this.RedisManager.clearProjectState(this.project_id, done) + this.RedisManager.clearProjectState(this.project_id, done) }) - return it('should delete the project state', function () { - return this.rclient.del + it('should delete the project state', function () { + this.rclient.del .calledWith(`ProjectState:${this.project_id}`) .should.equal(true) }) }) - return describe('renameDoc', function () { + describe('renameDoc', function () { beforeEach(function () { this.rclient.rpush = sinon.stub().yields() this.rclient.set = sinon.stub().yields() - return (this.update = { - id: this.doc_id, + this.update = { + id: this.docId, pathname: (this.pathname = 'pathname'), newPathname: (this.newPathname = 'new-pathname'), - }) + } }) describe('the document is cached in redis', function () { @@ -1082,9 +1040,9 @@ describe('RedisManager', function () { this.ProjectHistoryRedisManager.queueRenameEntity = sinon .stub() .yields() - return this.RedisManager.renameDoc( + this.RedisManager.renameDoc( this.project_id, - this.doc_id, + this.docId, this.userId, this.update, this.projectHistoryId, @@ -1093,8 +1051,8 @@ describe('RedisManager', function () { }) it('update the cached pathname', function () { - return this.rclient.set - .calledWith(`Pathname:${this.doc_id}`, this.newPathname) + this.rclient.set + .calledWith(`Pathname:${this.docId}`, this.newPathname) .should.equal(true) }) }) @@ -1107,9 +1065,9 @@ describe('RedisManager', function () { this.ProjectHistoryRedisManager.queueRenameEntity = sinon .stub() .yields() - return this.RedisManager.renameDoc( + this.RedisManager.renameDoc( this.project_id, - this.doc_id, + this.docId, this.userId, this.update, this.projectHistoryId, @@ -1118,13 +1076,13 @@ describe('RedisManager', function () { }) it('does not update the cached pathname', function () { - return this.rclient.set.called.should.equal(false) + this.rclient.set.called.should.equal(false) }) }) - return describe('getDocVersion', function () { + describe('getDocVersion', function () { beforeEach(function () { - return (this.version = 12345) + this.version = 12345 }) describe('when the document does not have a project history type set', function () { @@ -1132,34 +1090,34 @@ describe('RedisManager', function () { this.rclient.mget = sinon .stub() .withArgs( - `DocVersion:${this.doc_id}`, - `ProjectHistoryType:${this.doc_id}` + `DocVersion:${this.docId}`, + `ProjectHistoryType:${this.docId}` ) .callsArgWith(2, null, [`${this.version}`]) - return this.RedisManager.getDocVersion(this.doc_id, this.callback) + this.RedisManager.getDocVersion(this.docId, this.callback) }) - return it('should return the document version and an undefined history type', function () { - return this.callback + it('should return the document version and an undefined history type', function () { + this.callback .calledWithExactly(null, this.version, undefined) .should.equal(true) }) }) - return describe('when the document has a project history type set', function () { + describe('when the document has a project history type set', function () { beforeEach(function () { this.rclient.mget = sinon .stub() .withArgs( - `DocVersion:${this.doc_id}`, - `ProjectHistoryType:${this.doc_id}` + `DocVersion:${this.docId}`, + `ProjectHistoryType:${this.docId}` ) .callsArgWith(2, null, [`${this.version}`, 'project-history']) - return this.RedisManager.getDocVersion(this.doc_id, this.callback) + this.RedisManager.getDocVersion(this.docId, this.callback) }) - return it('should return the document version and history type', function () { - return this.callback + it('should return the document version and history type', function () { + this.callback .calledWithExactly(null, this.version, 'project-history') .should.equal(true) })