Merge pull request #13264 from overleaf/msm-docupdater-cleanup

[docupdater] Cleanup track-changes code

GitOrigin-RevId: 906a4f57a9e7348f47af67cff44a32ae1bb2debb

services/document-updater/app/js/DocumentManager.js

@@ -42,15 +42,7 @@ module.exports = DocumentManager = {
           PersistenceManager.getDoc(
             projectId,
             docId,
-            (
-              error,
-              lines,
-              version,
-              ranges,
-              pathname,
-              projectHistoryId,
-              projectHistoryType
-            ) => {
+            (error, lines, version, ranges, pathname, projectHistoryId) => {
               if (error) {
                 return callback(error)
               }
@@ -62,7 +54,6 @@ module.exports = DocumentManager = {
                   version,
                   pathname,
                   projectHistoryId,
-                  projectHistoryType,
                 },
                 'got doc from persistence API'
               )
@@ -78,24 +69,15 @@ module.exports = DocumentManager = {
                   if (error) {
                     return callback(error)
                   }
-                  RedisManager.setHistoryType(
-                    docId,
-                    projectHistoryType,
-                    error => {
-                      if (error) {
-                        return callback(error)
-                      }
-                      callback(
-                        null,
-                        lines,
-                        version,
-                        ranges || {},
-                        pathname,
-                        projectHistoryId,
-                        null,
-                        false
-                      )
-                    }
+                  callback(
+                    null,
+                    lines,
+                    version,
+                    ranges || {},
+                    pathname,
+                    projectHistoryId,
+                    null,
+                    false
                   )
                 }
               )
@@ -358,9 +340,6 @@ module.exports = DocumentManager = {
         }
       }
 
-      // Flush in the background since it requires a http request
-      HistoryManager.flushDocChangesAsync(projectId, docId)
-
       RedisManager.removeDocFromMemory(projectId, docId, error => {
         if (error) {
           return callback(error)

services/document-updater/app/js/HistoryManager.js

@@ -1,82 +1,15 @@
-// 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
- */
 let HistoryManager
 const async = require('async')
 const logger = require('@overleaf/logger')
 const request = require('request')
 const Settings = require('@overleaf/settings')
-const HistoryRedisManager = require('./HistoryRedisManager')
 const ProjectHistoryRedisManager = require('./ProjectHistoryRedisManager')
-const RedisManager = require('./RedisManager')
 const metrics = require('./Metrics')
 
 module.exports = HistoryManager = {
-  flushDocChangesAsync(projectId, docId) {
-    if (
-      (Settings.apis != null ? Settings.apis.trackchanges : undefined) == null
-    ) {
-      logger.warn(
-        { docId },
-        'track changes API is not configured, so not flushing'
-      )
-      return
-    }
-    if (Settings.disableTrackChanges) {
-      logger.debug({ docId }, 'track changes is disabled, so not flushing')
-      return
-    }
-    return RedisManager.getHistoryType(
-      docId,
-      function (err, projectHistoryType) {
-        if (err != null) {
-          logger.warn({ err, docId }, 'error getting history type')
-        }
-        // if there's an error continue and flush to track-changes for safety
-        if (
-          Settings.disableDoubleFlush &&
-          projectHistoryType === 'project-history'
-        ) {
-          return logger.debug(
-            { docId, projectHistoryType },
-            'skipping track-changes flush'
-          )
-        } else {
-          metrics.inc('history-flush', 1, { status: 'track-changes' })
-          const url = `${Settings.apis.trackchanges.url}/project/${projectId}/doc/${docId}/flush`
-          logger.debug(
-            { projectId, docId, url, projectHistoryType },
-            'flushing doc in track changes api'
-          )
-          return request.post(url, function (error, res, body) {
-            if (error != null) {
-              return logger.error(
-                { error, docId, projectId },
-                'track changes doc to track changes api'
-              )
-            } else if (res.statusCode < 200 && res.statusCode >= 300) {
-              return logger.error(
-                { docId, projectId },
-                `track changes api returned a failure status code: ${res.statusCode}`
-              )
-            }
-          })
-        }
-      }
-    )
-  },
-
   // flush changes in the background
   flushProjectChangesAsync(projectId) {
-    if (!Settings.apis?.project_history?.enabled) {
-      return
-    }
-    return HistoryManager.flushProjectChanges(
+    HistoryManager.flushProjectChanges(
       projectId,
       { background: true },
       function () {}
@@ -88,9 +21,6 @@ module.exports = HistoryManager = {
     if (callback == null) {
       callback = function () {}
     }
-    if (!Settings.apis?.project_history?.enabled) {
-      return callback()
-    }
     if (options.skip_history_flush) {
       logger.debug({ projectId }, 'skipping flush of project history')
       return callback()
@@ -102,21 +32,18 @@ module.exports = HistoryManager = {
       qs.background = true
     } // pass on the background flush option if present
     logger.debug({ projectId, url, qs }, 'flushing doc in project history api')
-    return request.post({ url, qs }, function (error, res, body) {
-      if (error != null) {
-        logger.error(
-          { error, projectId },
-          'project history doc to track changes api'
-        )
-        return callback(error)
+    request.post({ url, qs }, function (error, res, body) {
+      if (error) {
+        logger.error({ error, projectId }, 'project history api request failed')
+        callback(error)
       } else if (res.statusCode < 200 && res.statusCode >= 300) {
         logger.error(
           { projectId },
           `project history api returned a failure status code: ${res.statusCode}`
         )
-        return callback(error)
+        callback(error)
       } else {
-        return callback()
+        callback()
       }
     })
   },
@@ -124,80 +51,30 @@ module.exports = HistoryManager = {
   FLUSH_DOC_EVERY_N_OPS: 100,
   FLUSH_PROJECT_EVERY_N_OPS: 500,
 
-  recordAndFlushHistoryOps(
-    projectId,
-    docId,
-    ops,
-    docOpsLength,
-    projectOpsLength,
-    callback
-  ) {
+  recordAndFlushHistoryOps(projectId, ops, projectOpsLength) {
     if (ops == null) {
       ops = []
     }
-    if (callback == null) {
-      callback = function () {}
-    }
     if (ops.length === 0) {
-      return callback()
+      return
     }
 
     // record updates for project history
-    if (Settings.apis?.project_history?.enabled) {
-      if (
-        HistoryManager.shouldFlushHistoryOps(
-          projectOpsLength,
-          ops.length,
-          HistoryManager.FLUSH_PROJECT_EVERY_N_OPS
-        )
-      ) {
-        // Do this in the background since it uses HTTP and so may be too
-        // slow to wait for when processing a doc update.
-        logger.debug(
-          { projectOpsLength, projectId },
-          'flushing project history api'
-        )
-        HistoryManager.flushProjectChangesAsync(projectId)
-      }
-    }
-
-    // if the doc_ops_length is undefined it means the project is not using track-changes
-    // so we can bail out here
-    if (Settings.disableTrackChanges || typeof docOpsLength === 'undefined') {
+    if (
+      HistoryManager.shouldFlushHistoryOps(
+        projectOpsLength,
+        ops.length,
+        HistoryManager.FLUSH_PROJECT_EVERY_N_OPS
+      )
+    ) {
+      // Do this in the background since it uses HTTP and so may be too
+      // slow to wait for when processing a doc update.
       logger.debug(
-        { projectId, docId },
-        'skipping flush to track-changes, only using project-history'
+        { projectOpsLength, projectId },
+        'flushing project history api'
       )
-      return callback()
+      HistoryManager.flushProjectChangesAsync(projectId)
     }
-
-    // record updates for track-changes
-    return HistoryRedisManager.recordDocHasHistoryOps(
-      projectId,
-      docId,
-      ops,
-      function (error) {
-        if (error != null) {
-          return callback(error)
-        }
-        if (
-          HistoryManager.shouldFlushHistoryOps(
-            docOpsLength,
-            ops.length,
-            HistoryManager.FLUSH_DOC_EVERY_N_OPS
-          )
-        ) {
-          // Do this in the background since it uses HTTP and so may be too
-          // slow to wait for when processing a doc update.
-          logger.debug(
-            { docOpsLength, docId, projectId },
-            'flushing track changes api'
-          )
-          HistoryManager.flushDocChangesAsync(projectId, docId)
-        }
-        return callback()
-      }
-    )
   },
 
   shouldFlushHistoryOps(length, opsLength, threshold) {
@@ -217,13 +94,13 @@ module.exports = HistoryManager = {
   MAX_PARALLEL_REQUESTS: 4,
 
   resyncProjectHistory(projectId, projectHistoryId, docs, files, callback) {
-    return ProjectHistoryRedisManager.queueResyncProjectStructure(
+    ProjectHistoryRedisManager.queueResyncProjectStructure(
       projectId,
       projectHistoryId,
       docs,
       files,
       function (error) {
-        if (error != null) {
+        if (error) {
           return callback(error)
         }
         const DocumentManager = require('./DocumentManager')
@@ -235,7 +112,7 @@ module.exports = HistoryManager = {
             cb
           )
         }
-        return async.eachLimit(
+        async.eachLimit(
           docs,
           HistoryManager.MAX_PARALLEL_REQUESTS,
           resyncDoc,

services/document-updater/app/js/HistoryRedisManager.js

@@ -1,41 +0,0 @@
-/* eslint-disable
-    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
- */
-let HistoryRedisManager
-const Settings = require('@overleaf/settings')
-const { rclient } = require('./RedisManager') // docsWithHistoryOps lives in main redis
-const Keys = Settings.redis.history.key_schema
-const logger = require('@overleaf/logger')
-
-module.exports = HistoryRedisManager = {
-  recordDocHasHistoryOps(projectId, docId, ops, callback) {
-    if (ops == null) {
-      ops = []
-    }
-    if (callback == null) {
-      callback = function () {}
-    }
-    if (ops.length === 0) {
-      return callback(new Error('cannot push no ops')) // This should never be called with no ops, but protect against a redis error if we sent an empty array to rpush
-    }
-    logger.debug({ projectId, docId }, 'marking doc in project for history ops')
-    return rclient.sadd(
-      Keys.docsWithHistoryOps({ project_id: projectId }),
-      docId,
-      function (error) {
-        if (error != null) {
-          return callback(error)
-        }
-        return callback()
-      }
-    )
-  },
-}

services/document-updater/app/js/PersistenceManager.js

@@ -99,8 +99,7 @@ function getDoc(projectId, docId, options = {}, _callback) {
         body.version,
         body.ranges,
         body.pathname,
-        body.projectHistoryId?.toString(),
-        body.projectHistoryType
+        body.projectHistoryId?.toString()
       )
     } else if (res.statusCode === 404) {
       callback(new Errors.NotFoundError(`doc not not found: ${urlPath}`))

services/document-updater/app/js/RedisManager.js

@@ -26,7 +26,6 @@ const MEGABYTES = 1024 * 1024
 const MAX_RANGES_SIZE = 3 * MEGABYTES
 
 const keys = Settings.redis.documentupdater.key_schema
-const historyKeys = Settings.redis.history.key_schema // note: this is track changes, not project-history
 
 module.exports = RedisManager = {
   rclient,
@@ -129,7 +128,6 @@ module.exports = RedisManager = {
       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 })
@@ -266,18 +264,14 @@ module.exports = RedisManager = {
   },
 
   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] = result || []
-        version = parseInt(version, 10)
-        callback(null, version, projectHistoryType)
+    rclient.mget(keys.docVersion({ doc_id: docId }), (error, result) => {
+      if (error) {
+        return callback(error)
       }
-    )
+      let [version] = result || []
+      version = parseInt(version, 10)
+      callback(null, version)
+    })
   },
 
   getDocLines(docId, callback) {
@@ -353,26 +347,6 @@ module.exports = RedisManager = {
     })
   },
 
-  getHistoryType(docId, callback) {
-    rclient.get(
-      keys.projectHistoryType({ doc_id: docId }),
-      (error, projectHistoryType) => {
-        if (error) {
-          return callback(error)
-        }
-        callback(null, projectHistoryType)
-      }
-    )
-  },
-
-  setHistoryType(docId, projectHistoryType, callback) {
-    rclient.set(
-      keys.projectHistoryType({ doc_id: docId }),
-      projectHistoryType,
-      callback
-    )
-  },
-
   DOC_OPS_TTL: 60 * minutes,
   DOC_OPS_MAX_LENGTH: 100,
   updateDocument(
@@ -388,163 +362,129 @@ module.exports = RedisManager = {
     if (appliedOps == null) {
       appliedOps = []
     }
-    RedisManager.getDocVersion(
-      docId,
-      (error, currentVersion, projectHistoryType) => {
-        if (error) {
-          return callback(error)
-        }
-        if (currentVersion + appliedOps.length !== newVersion) {
-          error = new Error(`Version mismatch. '${docId}' is corrupted.`)
-          logger.error(
-            {
-              err: error,
-              docId,
-              currentVersion,
-              newVersion,
-              opsLength: appliedOps.length,
-            },
-            'version mismatch'
-          )
+    RedisManager.getDocVersion(docId, (error, currentVersion) => {
+      if (error) {
+        return callback(error)
+      }
+      if (currentVersion + appliedOps.length !== newVersion) {
+        error = new Error(`Version mismatch. '${docId}' is corrupted.`)
+        logger.error(
+          {
+            err: error,
+            docId,
+            currentVersion,
+            newVersion,
+            opsLength: appliedOps.length,
+          },
+          'version mismatch'
+        )
+        return callback(error)
+      }
+
+      const jsonOps = appliedOps.map(op => JSON.stringify(op))
+      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, docId, jsonOps }, error.message)
           return callback(error)
         }
+      }
 
-        const jsonOps = appliedOps.map(op => JSON.stringify(op))
-        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, docId, jsonOps }, error.message)
-            return callback(error)
-          }
-        }
+      const newDocLines = JSON.stringify(docLines)
+      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, docId, newDocLines }, error.message)
+        return callback(error)
+      }
+      // Do an optimised size check on the docLines using the serialised
+      // length as an upper bound
+      const sizeBound = newDocLines.length
+      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({ projectId, docId, err, docSize }, err.message)
+        return callback(err)
+      }
+      const newHash = RedisManager._computeHash(newDocLines)
 
-        const newDocLines = JSON.stringify(docLines)
-        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, docId, newDocLines }, error.message)
+      const opVersions = appliedOps.map(op => op?.v)
+      logger.debug(
+        {
+          docId,
+          version: newVersion,
+          hash: newHash,
+          opVersions,
+        },
+        'updating doc in redis'
+      )
+      // record bytes sent to redis in update
+      metrics.summary('redis.docLines', newDocLines.length, {
+        status: 'update',
+      })
+      RedisManager._serializeRanges(ranges, (error, ranges) => {
+        if (error) {
+          logger.error({ err: error, docId }, error.message)
           return callback(error)
         }
-        // Do an optimised size check on the docLines using the serialised
-        // length as an upper bound
-        const sizeBound = newDocLines.length
-        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({ projectId, docId, err, docSize }, err.message)
-          return callback(err)
+        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, docId, ranges }, error.message)
+          return callback(error)
         }
-        const newHash = RedisManager._computeHash(newDocLines)
-
-        const opVersions = appliedOps.map(op => op?.v)
-        logger.debug(
-          {
-            docId,
-            version: newVersion,
-            hash: newHash,
-            opVersions,
-          },
-          'updating doc in redis'
-        )
-        // record bytes sent to redis in update
-        metrics.summary('redis.docLines', newDocLines.length, {
-          status: 'update',
+        const multi = rclient.multi()
+        multi.mset({
+          [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,
         })
-        RedisManager._serializeRanges(ranges, (error, ranges) => {
+        multi.ltrim(
+          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: 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: docId }), RedisManager.DOC_OPS_TTL) // index 6
+          // 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: docId }), Date.now(), 'NX')
+        }
+        multi.exec((error, result) => {
           if (error) {
-            logger.error({ err: error, docId }, error.message)
             return callback(error)
           }
-          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, docId, ranges }, error.message)
-            return callback(error)
-          }
-          const multi = rclient.multi()
-          multi.mset({
-            [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: 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: 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: docId }),
-              RedisManager.DOC_OPS_TTL
-            ) // index 6
-            if (
-              Settings.disableTrackChanges ||
-              projectHistoryType === 'project-history'
-            ) {
-              metrics.inc('history-queue', 1, { status: 'skip-track-changes' })
-              logger.debug(
-                { 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: 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: docId }), Date.now(), 'NX')
-          }
-          multi.exec((error, result) => {
-            if (error) {
-              return callback(error)
-            }
-
-            let docUpdateCount
-            if (
-              Settings.disableTrackChanges ||
-              projectHistoryType === 'project-history'
-            ) {
-              docUpdateCount = undefined // only using project history, don't bother with track-changes
-            } else {
-              // project is using old track-changes history service
-              docUpdateCount = result[4]
-            }
 
-            if (jsonOps.length > 0 && Settings.apis?.project_history?.enabled) {
-              metrics.inc('history-queue', 1, { status: 'project-history' })
-              ProjectHistoryRedisManager.queueOps(
-                projectId,
-                ...jsonOps,
-                (error, projectUpdateCount) => {
-                  if (error) {
-                    // The full project history can re-sync a project in case
-                    //  updates went missing.
-                    // Just record the error here and acknowledge the write-op.
-                    metrics.inc('history-queue-error')
-                  }
-                  callback(null, docUpdateCount, projectUpdateCount)
+          if (jsonOps.length > 0) {
+            metrics.inc('history-queue', 1, { status: 'project-history' })
+            ProjectHistoryRedisManager.queueOps(
+              projectId,
+              ...jsonOps,
+              (error, projectUpdateCount) => {
+                if (error) {
+                  // The full project history can re-sync a project in case
+                  //  updates went missing.
+                  // Just record the error here and acknowledge the write-op.
+                  metrics.inc('history-queue-error')
                 }
-              )
-            } else {
-              callback(null, docUpdateCount)
-            }
-          })
+                callback(null, projectUpdateCount)
+              }
+            )
+          } else {
+            callback(null)
+          }
         })
-      }
-    )
+      })
+    })
   },
 
   renameDoc(projectId, docId, userId, update, projectHistoryId, callback) {

services/document-updater/app/js/UpdateManager.js

@@ -6,10 +6,8 @@
 /*
  * decaffeinate suggestions:
  * DS101: Remove unnecessary use of Array.from
- * DS102: Remove unnecessary code created because of implicit returns
  * DS201: Simplify complex destructure assignments
  * 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 UpdateManager
@@ -31,44 +29,37 @@ const Profiler = require('./Profiler')
 
 module.exports = UpdateManager = {
   processOutstandingUpdates(projectId, docId, callback) {
-    if (callback == null) {
+    if (!callback) {
       callback = function () {}
     }
     const timer = new Metrics.Timer('updateManager.processOutstandingUpdates')
-    return UpdateManager.fetchAndApplyUpdates(
-      projectId,
-      docId,
-      function (error) {
-        timer.done()
-        if (error != null) {
-          return callback(error)
-        }
-        return callback()
-      }
-    )
+    UpdateManager.fetchAndApplyUpdates(projectId, docId, function (error) {
+      timer.done()
+      callback(error)
+    })
   },
 
   processOutstandingUpdatesWithLock(projectId, docId, callback) {
-    if (callback == null) {
+    if (!callback) {
       callback = function () {}
     }
     const profile = new Profiler('processOutstandingUpdatesWithLock', {
       project_id: projectId,
       doc_id: docId,
     })
-    return LockManager.tryLock(docId, (error, gotLock, lockValue) => {
-      if (error != null) {
+    LockManager.tryLock(docId, (error, gotLock, lockValue) => {
+      if (error) {
         return callback(error)
       }
       if (!gotLock) {
         return callback()
       }
       profile.log('tryLock')
-      return UpdateManager.processOutstandingUpdates(
+      UpdateManager.processOutstandingUpdates(
         projectId,
         docId,
         function (error) {
-          if (error != null) {
+          if (error) {
             return UpdateManager._handleErrorInsideLock(
               docId,
               lockValue,
@@ -77,12 +68,12 @@ module.exports = UpdateManager = {
             )
           }
           profile.log('processOutstandingUpdates')
-          return LockManager.releaseLock(docId, lockValue, error => {
-            if (error != null) {
+          LockManager.releaseLock(docId, lockValue, error => {
+            if (error) {
               return callback(error)
             }
             profile.log('releaseLock').end()
-            return UpdateManager.continueProcessingUpdatesWithLock(
+            UpdateManager.continueProcessingUpdatesWithLock(
               projectId,
               docId,
               callback
@@ -94,59 +85,56 @@ module.exports = UpdateManager = {
   },
 
   continueProcessingUpdatesWithLock(projectId, docId, callback) {
-    if (callback == null) {
+    if (!callback) {
       callback = function () {}
     }
-    return RealTimeRedisManager.getUpdatesLength(docId, (error, length) => {
-      if (error != null) {
+    RealTimeRedisManager.getUpdatesLength(docId, (error, length) => {
+      if (error) {
         return callback(error)
       }
       if (length > 0) {
-        return UpdateManager.processOutstandingUpdatesWithLock(
+        UpdateManager.processOutstandingUpdatesWithLock(
           projectId,
           docId,
           callback
         )
       } else {
-        return callback()
+        callback()
       }
     })
   },
 
   fetchAndApplyUpdates(projectId, docId, callback) {
-    if (callback == null) {
+    if (!callback) {
       callback = function () {}
     }
     const profile = new Profiler('fetchAndApplyUpdates', {
       project_id: projectId,
       doc_id: docId,
     })
-    return RealTimeRedisManager.getPendingUpdatesForDoc(
-      docId,
-      (error, updates) => {
-        if (error != null) {
-          return callback(error)
-        }
-        logger.debug(
-          { projectId, docId, count: updates.length },
-          'processing updates'
-        )
-        if (updates.length === 0) {
-          return callback()
-        }
-        profile.log('getPendingUpdatesForDoc')
-        const doUpdate = (update, cb) =>
-          UpdateManager.applyUpdate(projectId, docId, update, function (err) {
-            profile.log('applyUpdate')
-            return cb(err)
-          })
-        const finalCallback = function (err) {
-          profile.log('async done').end()
-          return callback(err)
-        }
-        return async.eachSeries(updates, doUpdate, finalCallback)
+    RealTimeRedisManager.getPendingUpdatesForDoc(docId, (error, updates) => {
+      if (error) {
+        return callback(error)
       }
-    )
+      logger.debug(
+        { projectId, docId, count: updates.length },
+        'processing updates'
+      )
+      if (updates.length === 0) {
+        return callback()
+      }
+      profile.log('getPendingUpdatesForDoc')
+      const doUpdate = (update, cb) =>
+        UpdateManager.applyUpdate(projectId, docId, update, function (err) {
+          profile.log('applyUpdate')
+          cb(err)
+        })
+      const finalCallback = function (err) {
+        profile.log('async done').end()
+        callback(err)
+      }
+      async.eachSeries(updates, doUpdate, finalCallback)
+    })
   },
 
   applyUpdate(projectId, docId, update, _callback) {
@@ -154,7 +142,7 @@ module.exports = UpdateManager = {
       _callback = function () {}
     }
     const callback = function (error) {
-      if (error != null) {
+      if (error) {
         RealTimeRedisManager.sendData({
           project_id: projectId,
           doc_id: docId,
@@ -163,7 +151,7 @@ module.exports = UpdateManager = {
         profile.log('sendData')
       }
       profile.end()
-      return _callback(error)
+      _callback(error)
     }
 
     const profile = new Profiler('applyUpdate', {
@@ -172,12 +160,12 @@ module.exports = UpdateManager = {
     })
     UpdateManager._sanitizeUpdate(update)
     profile.log('sanitizeUpdate', { sync: true })
-    return DocumentManager.getDoc(
+    DocumentManager.getDoc(
       projectId,
       docId,
       function (error, lines, version, ranges, pathname, projectHistoryId) {
         profile.log('getDoc')
-        if (error != null) {
+        if (error) {
           return callback(error)
         }
         if (lines == null || version == null) {
@@ -187,7 +175,7 @@ module.exports = UpdateManager = {
         }
         const previousVersion = version
         const incomingUpdateVersion = update.v
-        return ShareJsUpdateManager.applyUpdate(
+        ShareJsUpdateManager.applyUpdate(
           projectId,
           docId,
           update,
@@ -199,10 +187,10 @@ module.exports = UpdateManager = {
               // doc version, otherwise getPreviousDocOps is called.
               sync: incomingUpdateVersion === previousVersion,
             })
-            if (error != null) {
+            if (error) {
               return callback(error)
             }
-            return RangesManager.applyUpdate(
+            RangesManager.applyUpdate(
               projectId,
               docId,
               ranges,
@@ -216,10 +204,10 @@ module.exports = UpdateManager = {
                   lines
                 )
                 profile.log('RangesManager.applyUpdate', { sync: true })
-                if (error != null) {
+                if (error) {
                   return callback(error)
                 }
-                return RedisManager.updateDocument(
+                RedisManager.updateDocument(
                   projectId,
                   docId,
                   updatedDocLines,
@@ -227,68 +215,61 @@ module.exports = UpdateManager = {
                   appliedOps,
                   newRanges,
                   update.meta,
-                  function (error, docOpsLength, projectOpsLength) {
+                  function (error, projectOpsLength) {
                     profile.log('RedisManager.updateDocument')
-                    if (error != null) {
+                    if (error) {
                       return callback(error)
                     }
-                    return HistoryManager.recordAndFlushHistoryOps(
+                    HistoryManager.recordAndFlushHistoryOps(
                       projectId,
-                      docId,
                       appliedOps,
-                      docOpsLength,
-                      projectOpsLength,
-                      function (error) {
-                        profile.log('recordAndFlushHistoryOps')
-                        if (error != null) {
-                          return callback(error)
-                        }
-                        if (rangesWereCollapsed) {
-                          Metrics.inc('doc-snapshot')
-                          logger.debug(
-                            {
-                              projectId,
-                              docId,
-                              previousVersion,
-                              lines,
-                              ranges,
-                              update,
-                            },
-                            'update collapsed some ranges, snapshotting previous content'
-                          )
-                          // Do this last, since it's a mongo call, and so potentially longest running
-                          // If it overruns the lock, it's ok, since all of our redis work is done
-                          return SnapshotManager.recordSnapshot(
-                            projectId,
-                            docId,
-                            previousVersion,
-                            pathname,
-                            lines,
-                            ranges,
-                            function (error) {
-                              if (error != null) {
-                                logger.error(
-                                  {
-                                    err: error,
-                                    projectId,
-                                    docId,
-                                    version,
-                                    lines,
-                                    ranges,
-                                  },
-                                  'error recording snapshot'
-                                )
-                                return callback(error)
-                              } else {
-                                return callback()
-                              }
-                            }
-                          )
-                        } else {
-                          return callback()
-                        }
-                      }
+                      projectOpsLength
                     )
+                    profile.log('recordAndFlushHistoryOps')
+                    if (rangesWereCollapsed) {
+                      Metrics.inc('doc-snapshot')
+                      logger.debug(
+                        {
+                          projectId,
+                          docId,
+                          previousVersion,
+                          lines,
+                          ranges,
+                          update,
+                        },
+                        'update collapsed some ranges, snapshotting previous content'
+                      )
+                      // Do this last, since it's a mongo call, and so potentially longest running
+                      // If it overruns the lock, it's ok, since all of our redis work is done
+                      SnapshotManager.recordSnapshot(
+                        projectId,
+                        docId,
+                        previousVersion,
+                        pathname,
+                        lines,
+                        ranges,
+                        function (error) {
+                          if (error) {
+                            logger.error(
+                              {
+                                err: error,
+                                projectId,
+                                docId,
+                                version,
+                                lines,
+                                ranges,
+                              },
+                              'error recording snapshot'
+                            )
+                            callback(error)
+                          } else {
+                            callback()
+                          }
+                        }
+                      )
+                    } else {
+                      callback()
+                    }
                   }
                 )
               }
@@ -309,14 +290,14 @@ module.exports = UpdateManager = {
     })
     return LockManager.getLock(docId, function (error, lockValue) {
       profile.log('getLock')
-      if (error != null) {
+      if (error) {
         return callback(error)
       }
-      return UpdateManager.processOutstandingUpdates(
+      UpdateManager.processOutstandingUpdates(
         projectId,
         docId,
         function (error) {
-          if (error != null) {
+          if (error) {
             return UpdateManager._handleErrorInsideLock(
               docId,
               lockValue,
@@ -325,12 +306,12 @@ module.exports = UpdateManager = {
             )
           }
           profile.log('processOutstandingUpdates')
-          return method(
+          method(
             projectId,
             docId,
             ...Array.from(args),
             function (error, ...responseArgs) {
-              if (error != null) {
+              if (error) {
                 return UpdateManager._handleErrorInsideLock(
                   docId,
                   lockValue,
@@ -339,34 +320,30 @@ module.exports = UpdateManager = {
                 )
               }
               profile.log('method')
-              return LockManager.releaseLock(
-                docId,
-                lockValue,
-                function (error) {
-                  if (error != null) {
-                    return callback(error)
-                  }
-                  profile.log('releaseLock').end()
-                  callback(null, ...Array.from(responseArgs))
-                  // We held the lock for a while so updates might have queued up
-                  return UpdateManager.continueProcessingUpdatesWithLock(
-                    projectId,
-                    docId,
-                    err => {
-                      if (err) {
-                        // The processing may fail for invalid user updates.
-                        // This can be very noisy, put them on level DEBUG
-                        //  and record a metric.
-                        Metrics.inc('background-processing-updates-error')
-                        logger.debug(
-                          { err, projectId, docId },
-                          'error processing updates in background'
-                        )
-                      }
-                    }
-                  )
+              LockManager.releaseLock(docId, lockValue, function (error) {
+                if (error) {
+                  return callback(error)
                 }
-              )
+                profile.log('releaseLock').end()
+                callback(null, ...Array.from(responseArgs))
+                // We held the lock for a while so updates might have queued up
+                UpdateManager.continueProcessingUpdatesWithLock(
+                  projectId,
+                  docId,
+                  err => {
+                    if (err) {
+                      // The processing may fail for invalid user updates.
+                      // This can be very noisy, put them on level DEBUG
+                      //  and record a metric.
+                      Metrics.inc('background-processing-updates-error')
+                      logger.debug(
+                        { err, projectId, docId },
+                        'error processing updates in background'
+                      )
+                    }
+                  }
+                )
+              })
             }
           )
         }
@@ -375,10 +352,10 @@ module.exports = UpdateManager = {
   },
 
   _handleErrorInsideLock(docId, lockValue, originalError, callback) {
-    if (callback == null) {
+    if (!callback) {
       callback = function () {}
     }
-    return LockManager.releaseLock(docId, lockValue, lockError =>
+    LockManager.releaseLock(docId, lockValue, lockError =>
       callback(originalError)
     )
   },

services/document-updater/config/settings.defaults.js

@@ -14,11 +14,7 @@ module.exports = {
       user: process.env.WEB_API_USER || 'sharelatex',
       pass: process.env.WEB_API_PASSWORD || 'password',
     },
-    trackchanges: {
-      url: `http://${process.env.TRACK_CHANGES_HOST || 'localhost'}:3015`,
-    },
     project_history: {
-      enabled: true,
       url: `http://${process.env.PROJECT_HISTORY_HOST || 'localhost'}:3054`,
     },
   },
@@ -44,14 +40,6 @@ module.exports = {
       maxRetriesPerRequest: parseInt(
         process.env.REDIS_MAX_RETRIES_PER_REQUEST || '20'
       ),
-      key_schema: {
-        uncompressedHistoryOps({ doc_id: docId }) {
-          return `UncompressedHistoryOps:{${docId}}`
-        },
-        docsWithHistoryOps({ project_id: projectId }) {
-          return `DocsWithHistoryOps:{${projectId}}`
-        },
-      },
     },
 
     project_history: {
@@ -137,9 +125,6 @@ module.exports = {
         projectHistoryId({ doc_id: docId }) {
           return `ProjectHistoryId:{${docId}}`
         },
-        projectHistoryType({ doc_id: docId }) {
-          return `ProjectHistoryType:{${docId}}`
-        },
         projectState({ project_id: projectId }) {
           return `ProjectState:{${projectId}}`
         },
@@ -181,8 +166,4 @@ module.exports = {
   continuousBackgroundFlush: process.env.CONTINUOUS_BACKGROUND_FLUSH === 'true',
 
   smoothingOffset: process.env.SMOOTHING_OFFSET || 1000, // milliseconds
-
-  disableDoubleFlush: process.env.DISABLE_DOUBLE_FLUSH === 'true', // don't flush track-changes for projects using project-history
-
-  disableTrackChanges: process.env.DISABLE_TRACK_CHANGES === 'true', // stop sending any updates to track-changes
 }

services/document-updater/test/acceptance/js/ApplyingUpdatesToADocTests.js

@@ -3,7 +3,6 @@
 /*
  * 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
  */
@@ -11,9 +10,6 @@ const sinon = require('sinon')
 const { expect } = require('chai')
 const async = require('async')
 const Settings = require('@overleaf/settings')
-const rclientHistory = require('@overleaf/redis-wrapper').createClient(
-  Settings.redis.history
-) // note: this is track changes, not project-history
 const rclientProjectHistory = require('@overleaf/redis-wrapper').createClient(
   Settings.redis.project_history
 )
@@ -21,10 +17,8 @@ const rclientDU = require('@overleaf/redis-wrapper').createClient(
   Settings.redis.documentupdater
 )
 const Keys = Settings.redis.documentupdater.key_schema
-const HistoryKeys = Settings.redis.history.key_schema
 const ProjectHistoryKeys = Settings.redis.project_history.key_schema
 
-const MockTrackChangesApi = require('./helpers/MockTrackChangesApi')
 const MockWebApi = require('./helpers/MockWebApi')
 const DocUpdaterClient = require('./helpers/DocUpdaterClient')
 const DocUpdaterApp = require('./helpers/DocUpdaterApp')
@@ -44,7 +38,7 @@ describe('Applying updates to a doc', function () {
       v: this.version,
     }
     this.result = ['one', 'one and a half', 'two', 'three']
-    return DocUpdaterApp.ensureRunning(done)
+    DocUpdaterApp.ensureRunning(done)
   })
 
   describe('when the document is not loaded', function () {
@@ -67,18 +61,17 @@ describe('Applying updates to a doc', function () {
           if (error != null) {
             throw error
           }
-          return setTimeout(done, 200)
+          setTimeout(done, 200)
         }
       )
-      return null
     })
 
     after(function () {
-      return MockWebApi.getDocument.restore()
+      MockWebApi.getDocument.restore()
     })
 
     it('should load the document from the web API', function () {
-      return MockWebApi.getDocument
+      MockWebApi.getDocument
         .calledWith(this.project_id, this.doc_id)
         .should.equal(true)
     })
@@ -88,38 +81,11 @@ describe('Applying updates to a doc', function () {
         this.project_id,
         this.doc_id,
         (error, res, doc) => {
-          if (error) return done(error)
+          if (error) done(error)
           doc.lines.should.deep.equal(this.result)
-          return done()
-        }
-      )
-      return null
-    })
-
-    it('should push the applied updates to the track changes api', function (done) {
-      rclientHistory.lrange(
-        HistoryKeys.uncompressedHistoryOps({ doc_id: this.doc_id }),
-        0,
-        -1,
-        (error, updates) => {
-          if (error != null) {
-            throw error
-          }
-          JSON.parse(updates[0]).op.should.deep.equal(this.update.op)
-          return rclientHistory.sismember(
-            HistoryKeys.docsWithHistoryOps({ project_id: this.project_id }),
-            this.doc_id,
-            (error, result) => {
-              if (error != null) {
-                throw error
-              }
-              result.should.equal(1)
-              return done()
-            }
-          )
+          done()
         }
       )
-      return null
     })
 
     it('should push the applied updates to the project history changes api', function (done) {
@@ -132,10 +98,9 @@ describe('Applying updates to a doc', function () {
             throw error
           }
           JSON.parse(updates[0]).op.should.deep.equal(this.update.op)
-          return done()
+          done()
         }
       )
-      return null
     })
 
     it('should set the first op timestamp', function (done) {
@@ -150,13 +115,12 @@ describe('Applying updates to a doc', function () {
           result = parseInt(result, 10)
           result.should.be.within(this.startTime, Date.now())
           this.firstOpTimestamp = result
-          return done()
+          done()
         }
       )
-      return null
     })
 
-    return describe('when sending another update', function () {
+    describe('when sending another update', function () {
       before(function (done) {
         this.timeout = 10000
         this.second_update = Object.create(this.update)
@@ -169,13 +133,12 @@ describe('Applying updates to a doc', function () {
             if (error != null) {
               throw error
             }
-            return setTimeout(done, 200)
+            setTimeout(done, 200)
           }
         )
-        return null
       })
 
-      return it('should not change the first op timestamp', function (done) {
+      it('should not change the first op timestamp', function (done) {
         rclientProjectHistory.get(
           ProjectHistoryKeys.projectHistoryFirstOpTimestamp({
             project_id: this.project_id,
@@ -186,10 +149,9 @@ describe('Applying updates to a doc', function () {
             }
             result = parseInt(result, 10)
             result.should.equal(this.firstOpTimestamp)
-            return done()
+            done()
           }
         )
-        return null
       })
     })
   })
@@ -210,7 +172,7 @@ describe('Applying updates to a doc', function () {
           throw error
         }
         sinon.spy(MockWebApi, 'getDocument')
-        return DocUpdaterClient.sendUpdate(
+        DocUpdaterClient.sendUpdate(
           this.project_id,
           this.doc_id,
           this.update,
@@ -218,19 +180,18 @@ describe('Applying updates to a doc', function () {
             if (error != null) {
               throw error
             }
-            return setTimeout(done, 200)
+            setTimeout(done, 200)
           }
         )
       })
-      return null
     })
 
     after(function () {
-      return MockWebApi.getDocument.restore()
+      MockWebApi.getDocument.restore()
     })
 
     it('should not need to call the web api', function () {
-      return MockWebApi.getDocument.called.should.equal(false)
+      MockWebApi.getDocument.called.should.equal(false)
     })
 
     it('should update the doc', function (done) {
@@ -240,35 +201,12 @@ describe('Applying updates to a doc', function () {
         (error, res, doc) => {
           if (error) return done(error)
           doc.lines.should.deep.equal(this.result)
-          return done()
-        }
-      )
-      return null
-    })
-
-    it('should push the applied updates to the track changes api', function (done) {
-      rclientHistory.lrange(
-        HistoryKeys.uncompressedHistoryOps({ doc_id: this.doc_id }),
-        0,
-        -1,
-        (error, updates) => {
-          if (error) return done(error)
-          JSON.parse(updates[0]).op.should.deep.equal(this.update.op)
-          return rclientHistory.sismember(
-            HistoryKeys.docsWithHistoryOps({ project_id: this.project_id }),
-            this.doc_id,
-            (error, result) => {
-              if (error) return done(error)
-              result.should.equal(1)
-              return done()
-            }
-          )
+          done()
         }
       )
-      return null
     })
 
-    return it('should push the applied updates to the project history changes api', function (done) {
+    it('should push the applied updates to the project history changes api', function (done) {
       rclientProjectHistory.lrange(
         ProjectHistoryKeys.projectHistoryOps({ project_id: this.project_id }),
         0,
@@ -276,10 +214,9 @@ describe('Applying updates to a doc', function () {
         (error, updates) => {
           if (error) return done(error)
           JSON.parse(updates[0]).op.should.deep.equal(this.update.op)
-          return done()
+          done()
         }
       )
-      return null
     })
   })
 
@@ -293,14 +230,13 @@ describe('Applying updates to a doc', function () {
       MockWebApi.insertDoc(this.project_id, this.doc_id, {
         lines: this.lines,
         version: this.version,
-        projectHistoryType: 'project-history',
       })
       DocUpdaterClient.preloadDoc(this.project_id, this.doc_id, error => {
         if (error != null) {
           throw error
         }
         sinon.spy(MockWebApi, 'getDocument')
-        return DocUpdaterClient.sendUpdate(
+        DocUpdaterClient.sendUpdate(
           this.project_id,
           this.doc_id,
           this.update,
@@ -308,15 +244,14 @@ describe('Applying updates to a doc', function () {
             if (error != null) {
               throw error
             }
-            return setTimeout(done, 200)
+            setTimeout(done, 200)
           }
         )
       })
-      return null
     })
 
     after(function () {
-      return MockWebApi.getDocument.restore()
+      MockWebApi.getDocument.restore()
     })
 
     it('should update the doc', function (done) {
@@ -326,27 +261,12 @@ describe('Applying updates to a doc', function () {
         (error, res, doc) => {
           if (error) return done(error)
           doc.lines.should.deep.equal(this.result)
-          return done()
-        }
-      )
-      return null
-    })
-
-    it('should not push any applied updates to the track changes api', function (done) {
-      rclientHistory.lrange(
-        HistoryKeys.uncompressedHistoryOps({ doc_id: this.doc_id }),
-        0,
-        -1,
-        (error, updates) => {
-          if (error) return done(error)
-          updates.length.should.equal(0)
-          return done()
+          done()
         }
       )
-      return null
     })
 
-    return it('should push the applied updates to the project history changes api', function (done) {
+    it('should push the applied updates to the project history changes api', function (done) {
       rclientProjectHistory.lrange(
         ProjectHistoryKeys.projectHistoryOps({ project_id: this.project_id }),
         0,
@@ -354,10 +274,9 @@ describe('Applying updates to a doc', function () {
         (error, updates) => {
           if (error) return done(error)
           JSON.parse(updates[0]).op.should.deep.equal(this.update.op)
-          return done()
+          done()
         }
       )
-      return null
     })
   })
 
@@ -387,7 +306,7 @@ describe('Applying updates to a doc', function () {
           { doc_id: this.doc_id, v: 10, op: [{ i: 'd', p: 10 }] },
         ]
         this.my_result = ['hello world', '', '']
-        return done()
+        done()
       })
 
       it('should be able to continue applying updates when the project has been deleted', function (done) {
@@ -395,7 +314,7 @@ describe('Applying updates to a doc', function () {
         const actions = []
         for (update of Array.from(this.updates.slice(0, 6))) {
           ;(update => {
-            return actions.push(callback =>
+            actions.push(callback =>
               DocUpdaterClient.sendUpdate(
                 this.project_id,
                 this.doc_id,
@@ -410,7 +329,7 @@ describe('Applying updates to a doc', function () {
         )
         for (update of Array.from(this.updates.slice(6))) {
           ;(update => {
-            return actions.push(callback =>
+            actions.push(callback =>
               DocUpdaterClient.sendUpdate(
                 this.project_id,
                 this.doc_id,
@@ -425,47 +344,19 @@ describe('Applying updates to a doc', function () {
           if (error != null) {
             throw error
           }
-          return DocUpdaterClient.getDoc(
+          DocUpdaterClient.getDoc(
             this.project_id,
             this.doc_id,
             (error, res, doc) => {
               if (error) return done(error)
               doc.lines.should.deep.equal(this.my_result)
-              return done()
+              done()
             }
           )
         })
-        return null
-      })
-
-      it('should push the applied updates to the track changes api', function (done) {
-        rclientHistory.lrange(
-          HistoryKeys.uncompressedHistoryOps({ doc_id: this.doc_id }),
-          0,
-          -1,
-          (error, updates) => {
-            if (error) return done(error)
-            updates = Array.from(updates).map(u => JSON.parse(u))
-            for (let i = 0; i < this.updates.length; i++) {
-              const appliedUpdate = this.updates[i]
-              appliedUpdate.op.should.deep.equal(updates[i].op)
-            }
-
-            return rclientHistory.sismember(
-              HistoryKeys.docsWithHistoryOps({ project_id: this.project_id }),
-              this.doc_id,
-              (error, result) => {
-                if (error) return done(error)
-                result.should.equal(1)
-                return done()
-              }
-            )
-          }
-        )
-        return null
       })
 
-      return it('should store the doc ops in the correct order', function (done) {
+      it('should store the doc ops in the correct order', function (done) {
         rclientDU.lrange(
           Keys.docOps({ doc_id: this.doc_id }),
           0,
@@ -477,14 +368,13 @@ describe('Applying updates to a doc', function () {
               const appliedUpdate = this.updates[i]
               appliedUpdate.op.should.deep.equal(updates[i].op)
             }
-            return done()
+            done()
           }
         )
-        return null
       })
     })
 
-    return describe('when older ops come in after the delete', function () {
+    describe('when older ops come in after the delete', function () {
       before(function (done) {
         ;[this.project_id, this.doc_id] = Array.from([
           DocUpdaterClient.randomId(),
@@ -504,15 +394,15 @@ describe('Applying updates to a doc', function () {
           { doc_id: this.doc_id, v: 0, op: [{ i: 'world', p: 1 }] },
         ]
         this.my_result = ['hello', 'world', '']
-        return done()
+        done()
       })
 
-      return it('should be able to continue applying updates when the project has been deleted', function (done) {
+      it('should be able to continue applying updates when the project has been deleted', function (done) {
         let update
         const actions = []
         for (update of Array.from(this.updates.slice(0, 5))) {
           ;(update => {
-            return actions.push(callback =>
+            actions.push(callback =>
               DocUpdaterClient.sendUpdate(
                 this.project_id,
                 this.doc_id,
@@ -527,7 +417,7 @@ describe('Applying updates to a doc', function () {
         )
         for (update of Array.from(this.updates.slice(5))) {
           ;(update => {
-            return actions.push(callback =>
+            actions.push(callback =>
               DocUpdaterClient.sendUpdate(
                 this.project_id,
                 this.doc_id,
@@ -542,17 +432,16 @@ describe('Applying updates to a doc', function () {
           if (error != null) {
             throw error
           }
-          return DocUpdaterClient.getDoc(
+          DocUpdaterClient.getDoc(
             this.project_id,
             this.doc_id,
             (error, res, doc) => {
               if (error) return done(error)
               doc.lines.should.deep.equal(this.my_result)
-              return done()
+              done()
             }
           )
         })
-        return null
       })
     })
   })
@@ -585,10 +474,9 @@ describe('Applying updates to a doc', function () {
           if (error != null) {
             throw error
           }
-          return setTimeout(done, 200)
+          setTimeout(done, 200)
         }
       )
-      return null
     })
 
     it('should not update the doc', function (done) {
@@ -598,17 +486,16 @@ describe('Applying updates to a doc', function () {
         (error, res, doc) => {
           if (error) return done(error)
           doc.lines.should.deep.equal(this.lines)
-          return done()
+          done()
         }
       )
-      return null
     })
 
-    return it('should send a message with an error', function () {
+    it('should send a message with an error', function () {
       this.messageCallback.called.should.equal(true)
       const [channel, message] = Array.from(this.messageCallback.args[0])
       channel.should.equal('applied-ops')
-      return JSON.parse(message).should.deep.include({
+      JSON.parse(message).should.deep.include({
         project_id: this.project_id,
         doc_id: this.doc_id,
         error: 'Delete component does not match',
@@ -616,61 +503,6 @@ describe('Applying updates to a doc', function () {
     })
   })
 
-  describe('with enough updates to flush to the track changes api', function () {
-    before(function (done) {
-      ;[this.project_id, this.doc_id] = Array.from([
-        DocUpdaterClient.randomId(),
-        DocUpdaterClient.randomId(),
-      ])
-      const updates = []
-      for (let v = 0; v <= 199; v++) {
-        // Should flush after 100 ops
-        updates.push({
-          doc_id: this.doc_id,
-          op: [{ i: v.toString(), p: 0 }],
-          v,
-        })
-      }
-
-      sinon.spy(MockTrackChangesApi, 'flushDoc')
-
-      MockWebApi.insertDoc(this.project_id, this.doc_id, {
-        lines: this.lines,
-        version: 0,
-      })
-
-      // Send updates in chunks to causes multiple flushes
-      const actions = []
-      for (let i = 0; i <= 19; i++) {
-        ;(i => {
-          return actions.push(cb => {
-            return DocUpdaterClient.sendUpdates(
-              this.project_id,
-              this.doc_id,
-              updates.slice(i * 10, (i + 1) * 10),
-              cb
-            )
-          })
-        })(i)
-      }
-      async.series(actions, error => {
-        if (error != null) {
-          throw error
-        }
-        return setTimeout(done, 2000)
-      })
-      return null
-    })
-
-    after(function () {
-      return MockTrackChangesApi.flushDoc.restore()
-    })
-
-    return it('should flush the doc twice', function () {
-      return MockTrackChangesApi.flushDoc.calledTwice.should.equal(true)
-    })
-  })
-
   describe('when there is no version in Mongo', function () {
     before(function (done) {
       ;[this.project_id, this.doc_id] = Array.from([
@@ -694,23 +526,21 @@ describe('Applying updates to a doc', function () {
           if (error != null) {
             throw error
           }
-          return setTimeout(done, 200)
+          setTimeout(done, 200)
         }
       )
-      return null
     })
 
-    return it('should update the doc (using version = 0)', function (done) {
+    it('should update the doc (using version = 0)', function (done) {
       DocUpdaterClient.getDoc(
         this.project_id,
         this.doc_id,
         (error, res, doc) => {
           if (error) return done(error)
           doc.lines.should.deep.equal(this.result)
-          return done()
+          done()
         }
       )
-      return null
     })
   })
 
@@ -750,8 +580,8 @@ describe('Applying updates to a doc', function () {
           if (error != null) {
             throw error
           }
-          return setTimeout(() => {
-            return DocUpdaterClient.sendUpdate(
+          setTimeout(() => {
+            DocUpdaterClient.sendUpdate(
               this.project_id,
               this.doc_id,
               {
@@ -772,13 +602,12 @@ describe('Applying updates to a doc', function () {
                 if (error != null) {
                   throw error
                 }
-                return setTimeout(done, 200)
+                setTimeout(done, 200)
               }
             )
           }, 200)
         }
       )
-      return null
     })
 
     it('should update the doc', function (done) {
@@ -788,24 +617,21 @@ describe('Applying updates to a doc', function () {
         (error, res, doc) => {
           if (error) return done(error)
           doc.lines.should.deep.equal(this.result)
-          return done()
+          done()
         }
       )
-      return null
     })
 
-    return it('should return a message about duplicate ops', function () {
+    it('should return a message about duplicate ops', function () {
       this.messageCallback.calledTwice.should.equal(true)
       this.messageCallback.args[0][0].should.equal('applied-ops')
       expect(JSON.parse(this.messageCallback.args[0][1]).op.dup).to.be.undefined
       this.messageCallback.args[1][0].should.equal('applied-ops')
-      return expect(
-        JSON.parse(this.messageCallback.args[1][1]).op.dup
-      ).to.equal(true)
+      expect(JSON.parse(this.messageCallback.args[1][1]).op.dup).to.equal(true)
     })
   })
 
-  return describe('when sending updates for a non-existing doc id', function () {
+  describe('when sending updates for a non-existing doc id', function () {
     before(function (done) {
       ;[this.project_id, this.doc_id] = Array.from([
         DocUpdaterClient.randomId(),
@@ -829,10 +655,9 @@ describe('Applying updates to a doc', function () {
           if (error != null) {
             throw error
           }
-          return setTimeout(done, 200)
+          setTimeout(done, 200)
         }
       )
-      return null
     })
 
     it('should not update or create a doc', function (done) {
@@ -842,17 +667,16 @@ describe('Applying updates to a doc', function () {
         (error, res, doc) => {
           if (error) return done(error)
           res.statusCode.should.equal(404)
-          return done()
+          done()
         }
       )
-      return null
     })
 
-    return it('should send a message with an error', function () {
+    it('should send a message with an error', function () {
       this.messageCallback.called.should.equal(true)
       const [channel, message] = Array.from(this.messageCallback.args[0])
       channel.should.equal('applied-ops')
-      return JSON.parse(message).should.deep.include({
+      JSON.parse(message).should.deep.include({
         project_id: this.project_id,
         doc_id: this.doc_id,
         error: `doc not not found: /project/${this.project_id}/doc/${this.doc_id}`,

services/document-updater/test/acceptance/js/DeletingADocumentTests.js

@@ -3,12 +3,10 @@
 /*
  * 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
  */
 const sinon = require('sinon')
-const MockTrackChangesApi = require('./helpers/MockTrackChangesApi')
 const MockProjectHistoryApi = require('./helpers/MockProjectHistoryApi')
 const MockWebApi = require('./helpers/MockWebApi')
 const DocUpdaterClient = require('./helpers/DocUpdaterClient')
@@ -30,14 +28,12 @@ describe('Deleting a document', function () {
     }
     this.result = ['one', 'one and a half', 'two', 'three']
 
-    sinon.spy(MockTrackChangesApi, 'flushDoc')
     sinon.spy(MockProjectHistoryApi, 'flushProject')
-    return DocUpdaterApp.ensureRunning(done)
+    DocUpdaterApp.ensureRunning(done)
   })
 
   after(function () {
-    MockTrackChangesApi.flushDoc.restore()
-    return MockProjectHistoryApi.flushProject.restore()
+    MockProjectHistoryApi.flushProject.restore()
   })
 
   describe('when the updated doc exists in the doc updater', function () {
@@ -53,49 +49,45 @@ describe('Deleting a document', function () {
         lines: this.lines,
         version: this.version,
       })
-      return DocUpdaterClient.preloadDoc(
-        this.project_id,
-        this.doc_id,
-        error => {
-          if (error != null) {
-            throw error
-          }
-          return DocUpdaterClient.sendUpdate(
-            this.project_id,
-            this.doc_id,
-            this.update,
-            error => {
-              if (error != null) {
-                throw error
-              }
-              return setTimeout(() => {
-                return DocUpdaterClient.deleteDoc(
-                  this.project_id,
-                  this.doc_id,
-                  (error, res, body) => {
-                    if (error) return done(error)
-                    this.statusCode = res.statusCode
-                    return setTimeout(done, 200)
-                  }
-                )
-              }, 200)
-            }
-          )
+      DocUpdaterClient.preloadDoc(this.project_id, this.doc_id, error => {
+        if (error != null) {
+          throw error
         }
-      )
+        DocUpdaterClient.sendUpdate(
+          this.project_id,
+          this.doc_id,
+          this.update,
+          error => {
+            if (error != null) {
+              throw error
+            }
+            setTimeout(() => {
+              DocUpdaterClient.deleteDoc(
+                this.project_id,
+                this.doc_id,
+                (error, res, body) => {
+                  if (error) return done(error)
+                  this.statusCode = res.statusCode
+                  setTimeout(done, 200)
+                }
+              )
+            }, 200)
+          }
+        )
+      })
     })
 
     after(function () {
       MockWebApi.setDocument.restore()
-      return MockWebApi.getDocument.restore()
+      MockWebApi.getDocument.restore()
     })
 
     it('should return a 204 status code', function () {
-      return this.statusCode.should.equal(204)
+      this.statusCode.should.equal(204)
     })
 
     it('should send the updated document and version to the web api', function () {
-      return MockWebApi.setDocument
+      MockWebApi.setDocument
         .calledWith(this.project_id, this.doc_id, this.result, this.version + 1)
         .should.equal(true)
     })
@@ -103,7 +95,7 @@ describe('Deleting a document', function () {
     it('should need to reload the doc if read again', function (done) {
       MockWebApi.getDocument.resetHistory()
       MockWebApi.getDocument.called.should.equals(false)
-      return DocUpdaterClient.getDoc(
+      DocUpdaterClient.getDoc(
         this.project_id,
         this.doc_id,
         (error, res, doc) => {
@@ -111,25 +103,19 @@ describe('Deleting a document', function () {
           MockWebApi.getDocument
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
-    it('should flush track changes', function () {
-      return MockTrackChangesApi.flushDoc
-        .calledWith(this.doc_id)
-        .should.equal(true)
-    })
-
-    return it('should flush project history', function () {
-      return MockProjectHistoryApi.flushProject
+    it('should flush project history', function () {
+      MockProjectHistoryApi.flushProject
         .calledWith(this.project_id)
         .should.equal(true)
     })
   })
 
-  return describe('when the doc is not in the doc updater', function () {
+  describe('when the doc is not in the doc updater', function () {
     before(function (done) {
       ;[this.project_id, this.doc_id] = Array.from([
         DocUpdaterClient.randomId(),
@@ -140,33 +126,33 @@ describe('Deleting a document', function () {
       })
       sinon.spy(MockWebApi, 'setDocument')
       sinon.spy(MockWebApi, 'getDocument')
-      return DocUpdaterClient.deleteDoc(
+      DocUpdaterClient.deleteDoc(
         this.project_id,
         this.doc_id,
         (error, res, body) => {
           if (error) return done(error)
           this.statusCode = res.statusCode
-          return setTimeout(done, 200)
+          setTimeout(done, 200)
         }
       )
     })
 
     after(function () {
       MockWebApi.setDocument.restore()
-      return MockWebApi.getDocument.restore()
+      MockWebApi.getDocument.restore()
     })
 
     it('should return a 204 status code', function () {
-      return this.statusCode.should.equal(204)
+      this.statusCode.should.equal(204)
     })
 
     it('should not need to send the updated document to the web api', function () {
-      return MockWebApi.setDocument.called.should.equal(false)
+      MockWebApi.setDocument.called.should.equal(false)
     })
 
     it('should need to reload the doc if read again', function (done) {
       MockWebApi.getDocument.called.should.equals(false)
-      return DocUpdaterClient.getDoc(
+      DocUpdaterClient.getDoc(
         this.project_id,
         this.doc_id,
         (error, res, doc) => {
@@ -174,19 +160,13 @@ describe('Deleting a document', function () {
           MockWebApi.getDocument
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
-          return done()
+          done()
         }
       )
     })
 
-    it('should flush track changes', function () {
-      return MockTrackChangesApi.flushDoc
-        .calledWith(this.doc_id)
-        .should.equal(true)
-    })
-
-    return it('should flush project history', function () {
-      return MockProjectHistoryApi.flushProject
+    it('should flush project history', function () {
+      MockProjectHistoryApi.flushProject
         .calledWith(this.project_id)
         .should.equal(true)
     })

services/document-updater/test/acceptance/js/DeletingAProjectTests.js

@@ -3,14 +3,12 @@
 /*
  * 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
  */
 const sinon = require('sinon')
 const async = require('async')
 
-const MockTrackChangesApi = require('./helpers/MockTrackChangesApi')
 const MockProjectHistoryApi = require('./helpers/MockProjectHistoryApi')
 const MockWebApi = require('./helpers/MockWebApi')
 const DocUpdaterClient = require('./helpers/DocUpdaterClient')
@@ -59,48 +57,43 @@ describe('Deleting a project', function () {
       })
     }
 
-    return DocUpdaterApp.ensureRunning(done)
+    DocUpdaterApp.ensureRunning(done)
   })
 
   describe('with documents which have been updated', function () {
     before(function (done) {
       sinon.spy(MockWebApi, 'setDocument')
-      sinon.spy(MockTrackChangesApi, 'flushDoc')
       sinon.spy(MockProjectHistoryApi, 'flushProject')
 
-      return async.series(
+      async.series(
         this.docs.map(doc => {
           return callback => {
-            return DocUpdaterClient.preloadDoc(
-              this.project_id,
-              doc.id,
-              error => {
-                if (error != null) {
-                  return callback(error)
-                }
-                return DocUpdaterClient.sendUpdate(
-                  this.project_id,
-                  doc.id,
-                  doc.update,
-                  error => {
-                    return callback(error)
-                  }
-                )
+            DocUpdaterClient.preloadDoc(this.project_id, doc.id, error => {
+              if (error != null) {
+                return callback(error)
               }
-            )
+              DocUpdaterClient.sendUpdate(
+                this.project_id,
+                doc.id,
+                doc.update,
+                error => {
+                  callback(error)
+                }
+              )
+            })
           }
         }),
         error => {
           if (error != null) {
             throw error
           }
-          return setTimeout(() => {
-            return DocUpdaterClient.deleteProject(
+          setTimeout(() => {
+            DocUpdaterClient.deleteProject(
               this.project_id,
               (error, res, body) => {
                 if (error) return done(error)
                 this.statusCode = res.statusCode
-                return done()
+                done()
               }
             )
           }, 200)
@@ -110,16 +103,15 @@ describe('Deleting a project', function () {
 
     after(function () {
       MockWebApi.setDocument.restore()
-      MockTrackChangesApi.flushDoc.restore()
-      return MockProjectHistoryApi.flushProject.restore()
+      MockProjectHistoryApi.flushProject.restore()
     })
 
     it('should return a 204 status code', function () {
-      return this.statusCode.should.equal(204)
+      this.statusCode.should.equal(204)
     })
 
     it('should send each document to the web api', function () {
-      return Array.from(this.docs).map(doc =>
+      Array.from(this.docs).map(doc =>
         MockWebApi.setDocument
           .calledWith(this.project_id, doc.id, doc.updatedLines)
           .should.equal(true)
@@ -128,13 +120,13 @@ describe('Deleting a project', function () {
 
     it('should need to reload the docs if read again', function (done) {
       sinon.spy(MockWebApi, 'getDocument')
-      return async.series(
+      async.series(
         this.docs.map(doc => {
           return callback => {
             MockWebApi.getDocument
               .calledWith(this.project_id, doc.id)
               .should.equal(false)
-            return DocUpdaterClient.getDoc(
+            DocUpdaterClient.getDoc(
               this.project_id,
               doc.id,
               (error, res, returnedDoc) => {
@@ -142,26 +134,20 @@ describe('Deleting a project', function () {
                 MockWebApi.getDocument
                   .calledWith(this.project_id, doc.id)
                   .should.equal(true)
-                return callback()
+                callback()
               }
             )
           }
         }),
         () => {
           MockWebApi.getDocument.restore()
-          return done()
+          done()
         }
       )
     })
 
-    it('should flush each doc in track changes', function () {
-      return Array.from(this.docs).map(doc =>
-        MockTrackChangesApi.flushDoc.calledWith(doc.id).should.equal(true)
-      )
-    })
-
-    return it('should flush each doc in project history', function () {
-      return MockProjectHistoryApi.flushProject
+    it('should flush each doc in project history', function () {
+      MockProjectHistoryApi.flushProject
         .calledWith(this.project_id)
         .should.equal(true)
     })
@@ -170,30 +156,25 @@ describe('Deleting a project', function () {
   describe('with the background=true parameter from realtime and no request to flush the queue', function () {
     before(function (done) {
       sinon.spy(MockWebApi, 'setDocument')
-      sinon.spy(MockTrackChangesApi, 'flushDoc')
       sinon.spy(MockProjectHistoryApi, 'flushProject')
 
-      return async.series(
+      async.series(
         this.docs.map(doc => {
           return callback => {
-            return DocUpdaterClient.preloadDoc(
-              this.project_id,
-              doc.id,
-              callback
-            )
+            DocUpdaterClient.preloadDoc(this.project_id, doc.id, callback)
           }
         }),
         error => {
           if (error != null) {
             throw error
           }
-          return setTimeout(() => {
-            return DocUpdaterClient.deleteProjectOnShutdown(
+          setTimeout(() => {
+            DocUpdaterClient.deleteProjectOnShutdown(
               this.project_id,
               (error, res, body) => {
                 if (error) return done(error)
                 this.statusCode = res.statusCode
-                return done()
+                done()
               }
             )
           }, 200)
@@ -203,58 +184,45 @@ describe('Deleting a project', function () {
 
     after(function () {
       MockWebApi.setDocument.restore()
-      MockTrackChangesApi.flushDoc.restore()
-      return MockProjectHistoryApi.flushProject.restore()
+      MockProjectHistoryApi.flushProject.restore()
     })
 
     it('should return a 204 status code', function () {
-      return this.statusCode.should.equal(204)
+      this.statusCode.should.equal(204)
     })
 
     it('should not send any documents to the web api', function () {
-      return MockWebApi.setDocument.called.should.equal(false)
-    })
-
-    it('should not flush any docs in track changes', function () {
-      return MockTrackChangesApi.flushDoc.called.should.equal(false)
+      MockWebApi.setDocument.called.should.equal(false)
     })
 
-    return it('should not flush to project history', function () {
-      return MockProjectHistoryApi.flushProject.called.should.equal(false)
+    it('should not flush to project history', function () {
+      MockProjectHistoryApi.flushProject.called.should.equal(false)
     })
   })
 
-  return describe('with the background=true parameter from realtime and a request to flush the queue', function () {
+  describe('with the background=true parameter from realtime and a request to flush the queue', function () {
     before(function (done) {
       sinon.spy(MockWebApi, 'setDocument')
-      sinon.spy(MockTrackChangesApi, 'flushDoc')
       sinon.spy(MockProjectHistoryApi, 'flushProject')
 
-      return async.series(
+      async.series(
         this.docs.map(doc => {
           return callback => {
-            return DocUpdaterClient.preloadDoc(
-              this.project_id,
-              doc.id,
-              callback
-            )
+            DocUpdaterClient.preloadDoc(this.project_id, doc.id, callback)
           }
         }),
         error => {
           if (error != null) {
             throw error
           }
-          return setTimeout(() => {
-            return DocUpdaterClient.deleteProjectOnShutdown(
+          setTimeout(() => {
+            DocUpdaterClient.deleteProjectOnShutdown(
               this.project_id,
               (error, res, body) => {
                 if (error) return done(error)
                 this.statusCode = res.statusCode
                 // after deleting the project and putting it in the queue, flush the queue
-                return setTimeout(
-                  () => DocUpdaterClient.flushOldProjects(done),
-                  2000
-                )
+                setTimeout(() => DocUpdaterClient.flushOldProjects(done), 2000)
               }
             )
           }, 200)
@@ -264,30 +232,23 @@ describe('Deleting a project', function () {
 
     after(function () {
       MockWebApi.setDocument.restore()
-      MockTrackChangesApi.flushDoc.restore()
-      return MockProjectHistoryApi.flushProject.restore()
+      MockProjectHistoryApi.flushProject.restore()
     })
 
     it('should return a 204 status code', function () {
-      return this.statusCode.should.equal(204)
+      this.statusCode.should.equal(204)
     })
 
     it('should send each document to the web api', function () {
-      return Array.from(this.docs).map(doc =>
+      Array.from(this.docs).map(doc =>
         MockWebApi.setDocument
           .calledWith(this.project_id, doc.id, doc.updatedLines)
           .should.equal(true)
       )
     })
 
-    it('should flush each doc in track changes', function () {
-      return Array.from(this.docs).map(doc =>
-        MockTrackChangesApi.flushDoc.calledWith(doc.id).should.equal(true)
-      )
-    })
-
-    return it('should flush to project history', function () {
-      return MockProjectHistoryApi.flushProject.called.should.equal(true)
+    it('should flush to project history', function () {
+      MockProjectHistoryApi.flushProject.called.should.equal(true)
     })
   })
 })

services/document-updater/test/acceptance/js/SettingADocumentTests.js

@@ -6,7 +6,6 @@ const docUpdaterRedis = require('@overleaf/redis-wrapper').createClient(
 )
 const Keys = Settings.redis.documentupdater.key_schema
 
-const MockTrackChangesApi = require('./helpers/MockTrackChangesApi')
 const MockProjectHistoryApi = require('./helpers/MockProjectHistoryApi')
 const MockWebApi = require('./helpers/MockWebApi')
 const DocUpdaterClient = require('./helpers/DocUpdaterClient')
@@ -35,14 +34,12 @@ describe('Setting a document', function () {
     this.source = 'dropbox'
     this.user_id = 'user-id-123'
 
-    sinon.spy(MockTrackChangesApi, 'flushDoc')
     sinon.spy(MockProjectHistoryApi, 'flushProject')
     sinon.spy(MockWebApi, 'setDocument')
     DocUpdaterApp.ensureRunning(done)
   })
 
   after(function () {
-    MockTrackChangesApi.flushDoc.restore()
     MockProjectHistoryApi.flushProject.restore()
     MockWebApi.setDocument.restore()
   })
@@ -92,7 +89,6 @@ describe('Setting a document', function () {
     })
 
     after(function () {
-      MockTrackChangesApi.flushDoc.resetHistory()
       MockProjectHistoryApi.flushProject.resetHistory()
       MockWebApi.setDocument.resetHistory()
     })
@@ -228,7 +224,6 @@ describe('Setting a document', function () {
     })
 
     after(function () {
-      MockTrackChangesApi.flushDoc.resetHistory()
       MockProjectHistoryApi.flushProject.resetHistory()
       MockWebApi.setDocument.resetHistory()
     })
@@ -247,10 +242,6 @@ describe('Setting a document', function () {
         .should.equal(true)
     })
 
-    it('should flush track changes', function () {
-      MockTrackChangesApi.flushDoc.calledWith(this.doc_id).should.equal(true)
-    })
-
     it('should flush project history', function () {
       MockProjectHistoryApi.flushProject
         .calledWith(this.project_id)
@@ -319,7 +310,6 @@ describe('Setting a document', function () {
       })
 
       after(function () {
-        MockTrackChangesApi.flushDoc.resetHistory()
         MockProjectHistoryApi.flushProject.resetHistory()
         MockWebApi.setDocument.resetHistory()
       })
@@ -332,10 +322,6 @@ describe('Setting a document', function () {
         MockWebApi.setDocument.called.should.equal(false)
       })
 
-      it('should not flush track changes', function () {
-        MockTrackChangesApi.flushDoc.called.should.equal(false)
-      })
-
       it('should not flush project history', function () {
         MockProjectHistoryApi.flushProject.called.should.equal(false)
       })
@@ -376,7 +362,6 @@ describe('Setting a document', function () {
     })
 
     after(function () {
-      MockTrackChangesApi.flushDoc.resetHistory()
       MockProjectHistoryApi.flushProject.resetHistory()
       MockWebApi.setDocument.resetHistory()
     })
@@ -458,7 +443,6 @@ describe('Setting a document', function () {
       })
 
       after(function () {
-        MockTrackChangesApi.flushDoc.resetHistory()
         MockProjectHistoryApi.flushProject.resetHistory()
         MockWebApi.setDocument.resetHistory()
       })
@@ -521,7 +505,6 @@ describe('Setting a document', function () {
       })
 
       after(function () {
-        MockTrackChangesApi.flushDoc.resetHistory()
         MockProjectHistoryApi.flushProject.resetHistory()
         MockWebApi.setDocument.resetHistory()
       })

services/document-updater/test/acceptance/js/helpers/MockTrackChangesApi.js

@@ -1,45 +0,0 @@
-// 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
- */
-let MockTrackChangesApi
-const express = require('express')
-const app = express()
-
-module.exports = MockTrackChangesApi = {
-  flushDoc(docId, callback) {
-    if (callback == null) {
-      callback = function () {}
-    }
-    return callback()
-  },
-
-  run() {
-    app.post('/project/:project_id/doc/:doc_id/flush', (req, res, next) => {
-      return this.flushDoc(req.params.doc_id, error => {
-        if (error != null) {
-          return res.sendStatus(500)
-        } else {
-          return res.sendStatus(204)
-        }
-      })
-    })
-
-    return app
-      .listen(3015, error => {
-        if (error != null) {
-          throw error
-        }
-      })
-      .on('error', error => {
-        console.error('error starting MockTrackChangesApi:', error.message)
-        return process.exit(1)
-      })
-  },
-}
-
-MockTrackChangesApi.run()

services/document-updater/test/stress/js/run.js

@@ -14,7 +14,6 @@
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
 const DocUpdaterClient = require('../../acceptance/js/helpers/DocUpdaterClient')
-// MockTrackChangesApi = require "../../acceptance/js/helpers/MockTrackChangesApi"
 // MockWebApi = require "../../acceptance/js/helpers/MockWebApi"
 const assert = require('assert')
 const async = require('async')

services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js

@@ -19,7 +19,6 @@ describe('DocumentManager', function () {
         './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}),
         './PersistenceManager': (this.PersistenceManager = {}),
         './HistoryManager': (this.HistoryManager = {
-          flushDocChangesAsync: sinon.stub(),
           flushProjectChangesAsync: sinon.stub(),
         }),
         './Metrics': this.Metrics,
@@ -32,7 +31,6 @@ describe('DocumentManager', function () {
     })
     this.project_id = 'project-id-123'
     this.projectHistoryId = 'history-id-123'
-    this.projectHistoryType = 'project-history'
     this.doc_id = 'doc-id-123'
     this.user_id = 1234
     this.callback = sinon.stub()
@@ -82,12 +80,6 @@ describe('DocumentManager', function () {
       it('should time the execution', function () {
         this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
-
-      it('should flush to the history api', function () {
-        this.HistoryManager.flushDocChangesAsync
-          .calledWithExactly(this.project_id, this.doc_id)
-          .should.equal(true)
-      })
     })
 
     describe('when a flush error occurs', function () {
@@ -388,11 +380,9 @@ describe('DocumentManager', function () {
             this.version,
             this.ranges,
             this.pathname,
-            this.projectHistoryId,
-            this.projectHistoryType
+            this.projectHistoryId
           )
         this.RedisManager.putDocInMemory = sinon.stub().yields()
-        this.RedisManager.setHistoryType = sinon.stub().yields()
         this.DocumentManager.getDoc(this.project_id, this.doc_id, this.callback)
       })
 
@@ -422,12 +412,6 @@ describe('DocumentManager', function () {
           .should.equal(true)
       })
 
-      it('should set the history type in Redis', function () {
-        this.RedisManager.setHistoryType
-          .calledWith(this.doc_id, this.projectHistoryType)
-          .should.equal(true)
-      })
-
       it('should call the callback with the doc info', function () {
         this.callback
           .calledWith(

services/document-updater/test/unit/js/HistoryManager/HistoryManagerTests.js

@@ -1,14 +1,6 @@
 /* eslint-disable
     mocha/no-nested-tests,
-    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 SandboxedModule = require('sandboxed-module')
 const sinon = require('sinon')
 const modulePath = require('path').join(
@@ -24,85 +16,18 @@ describe('HistoryManager', function () {
         '@overleaf/settings': (this.Settings = {
           apis: {
             project_history: {
-              enabled: true,
               url: 'http://project_history.example.com',
             },
-            trackchanges: {
-              url: 'http://trackchanges.example.com',
-            },
           },
         }),
         './DocumentManager': (this.DocumentManager = {}),
-        './HistoryRedisManager': (this.HistoryRedisManager = {}),
         './RedisManager': (this.RedisManager = {}),
         './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}),
         './Metrics': (this.metrics = { inc: sinon.stub() }),
       },
     })
     this.project_id = 'mock-project-id'
-    this.doc_id = 'mock-doc-id'
-    return (this.callback = sinon.stub())
-  })
-
-  describe('flushDocChangesAsync', function () {
-    beforeEach(function () {
-      return (this.request.post = sinon
-        .stub()
-        .callsArgWith(1, null, { statusCode: 204 }))
-    })
-
-    describe('when the project uses track changes', function () {
-      beforeEach(function () {
-        this.RedisManager.getHistoryType = sinon
-          .stub()
-          .yields(null, 'track-changes')
-        return this.HistoryManager.flushDocChangesAsync(
-          this.project_id,
-          this.doc_id
-        )
-      })
-
-      return it('should send a request to the track changes api', function () {
-        return this.request.post
-          .calledWith(
-            `${this.Settings.apis.trackchanges.url}/project/${this.project_id}/doc/${this.doc_id}/flush`
-          )
-          .should.equal(true)
-      })
-    })
-
-    describe('when the project uses project history and double flush is not disabled', function () {
-      beforeEach(function () {
-        this.RedisManager.getHistoryType = sinon
-          .stub()
-          .yields(null, 'project-history')
-        return this.HistoryManager.flushDocChangesAsync(
-          this.project_id,
-          this.doc_id
-        )
-      })
-
-      return it('should send a request to the track changes api', function () {
-        return this.request.post.called.should.equal(true)
-      })
-    })
-
-    return describe('when the project uses project history and double flush is disabled', function () {
-      beforeEach(function () {
-        this.Settings.disableDoubleFlush = true
-        this.RedisManager.getHistoryType = sinon
-          .stub()
-          .yields(null, 'project-history')
-        return this.HistoryManager.flushDocChangesAsync(
-          this.project_id,
-          this.doc_id
-        )
-      })
-
-      return it('should not send a request to the track changes api', function () {
-        return this.request.post.called.should.equal(false)
-      })
-    })
+    this.callback = sinon.stub()
   })
 
   describe('flushProjectChangesAsync', function () {
@@ -111,11 +36,11 @@ describe('HistoryManager', function () {
         .stub()
         .callsArgWith(1, null, { statusCode: 204 })
 
-      return this.HistoryManager.flushProjectChangesAsync(this.project_id)
+      this.HistoryManager.flushProjectChangesAsync(this.project_id)
     })
 
-    return it('should send a request to the project history api', function () {
-      return this.request.post
+    it('should send a request to the project history api', function () {
+      this.request.post
         .calledWith({
           url: `${this.Settings.apis.project_history.url}/project/${this.project_id}/flush`,
           qs: { background: true },
@@ -130,7 +55,7 @@ describe('HistoryManager', function () {
         this.request.post = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 204 })
-        return this.HistoryManager.flushProjectChanges(
+        this.HistoryManager.flushProjectChanges(
           this.project_id,
           {
             background: true,
@@ -139,8 +64,8 @@ describe('HistoryManager', function () {
         )
       })
 
-      return it('should send a request to the project history api', function () {
-        return this.request.post
+      it('should send a request to the project history api', function () {
+        this.request.post
           .calledWith({
             url: `${this.Settings.apis.project_history.url}/project/${this.project_id}/flush`,
             qs: { background: true },
@@ -149,10 +74,10 @@ describe('HistoryManager', function () {
       })
     })
 
-    return describe('with the skip_history_flush option', function () {
+    describe('with the skip_history_flush option', function () {
       beforeEach(function (done) {
         this.request.post = sinon.stub()
-        return this.HistoryManager.flushProjectChanges(
+        this.HistoryManager.flushProjectChanges(
           this.project_id,
           {
             skip_history_flush: true,
@@ -161,8 +86,8 @@ describe('HistoryManager', function () {
         )
       })
 
-      return it('should not send a request to the project history api', function () {
-        return this.request.post.called.should.equal(false)
+      it('should not send a request to the project history api', function () {
+        this.request.post.called.should.equal(false)
       })
     })
   })
@@ -171,45 +96,21 @@ describe('HistoryManager', function () {
     beforeEach(function () {
       this.ops = ['mock-ops']
       this.project_ops_length = 10
-      this.doc_ops_length = 5
 
       this.HistoryManager.flushProjectChangesAsync = sinon.stub()
-      this.HistoryRedisManager.recordDocHasHistoryOps = sinon.stub().callsArg(3)
-      return (this.HistoryManager.flushDocChangesAsync = sinon.stub())
     })
 
     describe('with no ops', function () {
       beforeEach(function () {
-        return this.HistoryManager.recordAndFlushHistoryOps(
+        this.HistoryManager.recordAndFlushHistoryOps(
           this.project_id,
-          this.doc_id,
           [],
-          this.doc_ops_length,
-          this.project_ops_length,
-          this.callback
+          this.project_ops_length
         )
       })
 
       it('should not flush project changes', function () {
-        return this.HistoryManager.flushProjectChangesAsync.called.should.equal(
-          false
-        )
-      })
-
-      it('should not record doc has history ops', function () {
-        return this.HistoryRedisManager.recordDocHasHistoryOps.called.should.equal(
-          false
-        )
-      })
-
-      it('should not flush doc changes', function () {
-        return this.HistoryManager.flushDocChangesAsync.called.should.equal(
-          false
-        )
-      })
-
-      return it('should call the callback', function () {
-        return this.callback.called.should.equal(true)
+        this.HistoryManager.flushProjectChangesAsync.called.should.equal(false)
       })
     })
 
@@ -219,43 +120,19 @@ describe('HistoryManager', function () {
         this.HistoryManager.shouldFlushHistoryOps
           .withArgs(this.project_ops_length)
           .returns(true)
-        this.HistoryManager.shouldFlushHistoryOps
-          .withArgs(this.doc_ops_length)
-          .returns(false)
 
-        return this.HistoryManager.recordAndFlushHistoryOps(
+        this.HistoryManager.recordAndFlushHistoryOps(
           this.project_id,
-          this.doc_id,
           this.ops,
-          this.doc_ops_length,
-          this.project_ops_length,
-          this.callback
+          this.project_ops_length
         )
       })
 
       it('should flush project changes', function () {
-        return this.HistoryManager.flushProjectChangesAsync
+        this.HistoryManager.flushProjectChangesAsync
           .calledWith(this.project_id)
           .should.equal(true)
       })
-
-      it('should record doc has history ops', function () {
-        return this.HistoryRedisManager.recordDocHasHistoryOps.calledWith(
-          this.project_id,
-          this.doc_id,
-          this.ops
-        )
-      })
-
-      it('should not flush doc changes', function () {
-        return this.HistoryManager.flushDocChangesAsync.called.should.equal(
-          false
-        )
-      })
-
-      return it('should call the callback', function () {
-        return this.callback.called.should.equal(true)
-      })
     })
 
     describe('with enough ops to flush doc changes', function () {
@@ -264,76 +141,22 @@ describe('HistoryManager', function () {
         this.HistoryManager.shouldFlushHistoryOps
           .withArgs(this.project_ops_length)
           .returns(false)
-        this.HistoryManager.shouldFlushHistoryOps
-          .withArgs(this.doc_ops_length)
-          .returns(true)
 
-        return this.HistoryManager.recordAndFlushHistoryOps(
+        this.HistoryManager.recordAndFlushHistoryOps(
           this.project_id,
-          this.doc_id,
           this.ops,
-          this.doc_ops_length,
-          this.project_ops_length,
-          this.callback
+          this.project_ops_length
         )
       })
 
       it('should not flush project changes', function () {
-        return this.HistoryManager.flushProjectChangesAsync.called.should.equal(
-          false
-        )
-      })
-
-      it('should record doc has history ops', function () {
-        return this.HistoryRedisManager.recordDocHasHistoryOps.calledWith(
-          this.project_id,
-          this.doc_id,
-          this.ops
-        )
-      })
-
-      it('should flush doc changes', function () {
-        return this.HistoryManager.flushDocChangesAsync
-          .calledWith(this.project_id, this.doc_id)
-          .should.equal(true)
-      })
-
-      return it('should call the callback', function () {
-        return this.callback.called.should.equal(true)
-      })
-    })
-
-    describe('when recording doc has history ops errors', function () {
-      beforeEach(function () {
-        this.error = new Error('error')
-        this.HistoryRedisManager.recordDocHasHistoryOps = sinon
-          .stub()
-          .callsArgWith(3, this.error)
-
-        return this.HistoryManager.recordAndFlushHistoryOps(
-          this.project_id,
-          this.doc_id,
-          this.ops,
-          this.doc_ops_length,
-          this.project_ops_length,
-          this.callback
-        )
-      })
-
-      it('should not flush doc changes', function () {
-        return this.HistoryManager.flushDocChangesAsync.called.should.equal(
-          false
-        )
-      })
-
-      return it('should call the callback with the error', function () {
-        return this.callback.calledWith(this.error).should.equal(true)
+        this.HistoryManager.flushProjectChangesAsync.called.should.equal(false)
       })
     })
 
-    return describe('shouldFlushHistoryOps', function () {
+    describe('shouldFlushHistoryOps', function () {
       it('should return false if the number of ops is not known', function () {
-        return this.HistoryManager.shouldFlushHistoryOps(
+        this.HistoryManager.shouldFlushHistoryOps(
           null,
           ['a', 'b', 'c'].length,
           1
@@ -354,18 +177,18 @@ describe('HistoryManager', function () {
         // Currently there are 15 ops
         // Previously we were on 12 ops
         // We've reached a new multiple of 5
-        return this.HistoryManager.shouldFlushHistoryOps(
+        this.HistoryManager.shouldFlushHistoryOps(
           15,
           ['a', 'b', 'c'].length,
           5
         ).should.equal(true)
       })
 
-      return it('should return true if the updates took past the threshold', function () {
+      it('should return true if the updates took past the threshold', function () {
         // Currently there are 19 ops
         // Previously we were on 16 ops
         // We didn't pass over a multiple of 5
-        return this.HistoryManager.shouldFlushHistoryOps(
+        this.HistoryManager.shouldFlushHistoryOps(
           17,
           ['a', 'b', 'c'].length,
           5
@@ -374,7 +197,7 @@ describe('HistoryManager', function () {
     })
   })
 
-  return describe('resyncProjectHistory', function () {
+  describe('resyncProjectHistory', function () {
     beforeEach(function () {
       this.projectHistoryId = 'history-id-1234'
       this.docs = [
@@ -394,7 +217,7 @@ describe('HistoryManager', function () {
         .stub()
         .yields()
       this.DocumentManager.resyncDocContentsWithLock = sinon.stub().yields()
-      return this.HistoryManager.resyncProjectHistory(
+      this.HistoryManager.resyncProjectHistory(
         this.project_id,
         this.projectHistoryId,
         this.docs,
@@ -404,7 +227,7 @@ describe('HistoryManager', function () {
     })
 
     it('should queue a project structure reync', function () {
-      return this.ProjectHistoryRedisManager.queueResyncProjectStructure
+      this.ProjectHistoryRedisManager.queueResyncProjectStructure
         .calledWith(
           this.project_id,
           this.projectHistoryId,
@@ -415,13 +238,13 @@ describe('HistoryManager', function () {
     })
 
     it('should queue doc content reyncs', function () {
-      return this.DocumentManager.resyncDocContentsWithLock
+      this.DocumentManager.resyncDocContentsWithLock
         .calledWith(this.project_id, this.docs[0].doc, this.docs[0].path)
         .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)
     })
   })
 })

services/document-updater/test/unit/js/HistoryRedisManager/HistoryRedisManagerTests.js

@@ -1,99 +0,0 @@
-/* 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
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const sinon = require('sinon')
-const modulePath = '../../../../app/js/HistoryRedisManager.js'
-const SandboxedModule = require('sandboxed-module')
-const Errors = require('../../../../app/js/Errors')
-
-describe('HistoryRedisManager', function () {
-  beforeEach(function () {
-    this.rclient = {
-      auth() {},
-      exec: sinon.stub(),
-    }
-    this.rclient.multi = () => this.rclient
-    this.HistoryRedisManager = SandboxedModule.require(modulePath, {
-      requires: {
-        './RedisManager': { rclient: this.rclient },
-        '@overleaf/settings': {
-          redis: {
-            history: (this.settings = {
-              key_schema: {
-                uncompressedHistoryOps({ doc_id: docId }) {
-                  return `UncompressedHistoryOps:${docId}`
-                },
-                docsWithHistoryOps({ project_id: projectId }) {
-                  return `DocsWithHistoryOps:${projectId}`
-                },
-              },
-            }),
-          },
-        },
-      },
-    })
-    this.doc_id = 'doc-id-123'
-    this.project_id = 'project-id-123'
-    return (this.callback = sinon.stub())
-  })
-
-  return describe('recordDocHasHistoryOps', function () {
-    beforeEach(function () {
-      this.ops = [{ op: [{ i: 'foo', p: 4 }] }, { op: [{ i: 'bar', p: 56 }] }]
-      return (this.rclient.sadd = sinon.stub().yields())
-    })
-
-    describe('with ops', function () {
-      beforeEach(function (done) {
-        return this.HistoryRedisManager.recordDocHasHistoryOps(
-          this.project_id,
-          this.doc_id,
-          this.ops,
-          (...args) => {
-            this.callback(...Array.from(args || []))
-            return done()
-          }
-        )
-      })
-
-      return it('should add the doc_id to the set of which records the project docs', function () {
-        return this.rclient.sadd
-          .calledWith(`DocsWithHistoryOps:${this.project_id}`, this.doc_id)
-          .should.equal(true)
-      })
-    })
-
-    return describe('with no ops', function () {
-      beforeEach(function (done) {
-        return this.HistoryRedisManager.recordDocHasHistoryOps(
-          this.project_id,
-          this.doc_id,
-          [],
-          (...args) => {
-            this.callback(...Array.from(args || []))
-            return done()
-          }
-        )
-      })
-
-      it('should not add the doc_id to the set of which records the project docs', function () {
-        return this.rclient.sadd.called.should.equal(false)
-      })
-
-      return it('should call the callback with an error', function () {
-        return this.callback
-          .calledWith(sinon.match.instanceOf(Error))
-          .should.equal(true)
-      })
-    })
-  })
-})

services/document-updater/test/unit/js/RedisManager/RedisManagerTests.js

@@ -15,9 +15,6 @@ describe('RedisManager', function () {
         './ProjectHistoryRedisManager': (this.ProjectHistoryRedisManager = {}),
         '@overleaf/settings': (this.settings = {
           documentupdater: { logHashErrors: { write: true, read: true } },
-          apis: {
-            project_history: { enabled: true },
-          },
           redis: {
             documentupdater: {
               key_schema: {
@@ -54,9 +51,6 @@ describe('RedisManager', function () {
                 projectHistoryId({ doc_id: docId }) {
                   return `ProjectHistoryId:${docId}`
                 },
-                projectHistoryType({ doc_id: docId }) {
-                  return `ProjectHistoryType:${docId}`
-                },
                 projectState({ project_id: projectId }) {
                   return `ProjectState:${projectId}`
                 },
@@ -71,16 +65,6 @@ describe('RedisManager', function () {
                 },
               },
             },
-            history: {
-              key_schema: {
-                uncompressedHistoryOps({ doc_id: docId }) {
-                  return `UncompressedHistoryOps:${docId}`
-                },
-                docsWithHistoryOps({ project_id: projectId }) {
-                  return `DocsWithHistoryOps:${projectId}`
-                },
-              },
-            },
           },
         }),
         '@overleaf/redis-wrapper': {
@@ -463,142 +447,94 @@ describe('RedisManager', function () {
     })
 
     describe('with a consistent version', function () {
-      beforeEach(function () {})
-
-      describe('with project history enabled', function () {
-        beforeEach(function () {
-          this.settings.apis.project_history.enabled = true
-          this.RedisManager.getDocVersion
-            .withArgs(this.docId)
-            .yields(null, this.version - this.ops.length)
-          this.RedisManager.updateDocument(
-            this.project_id,
-            this.docId,
-            this.lines,
-            this.version,
-            this.ops,
-            this.ranges,
-            this.updateMeta,
-            this.callback
-          )
-        })
-
-        it('should get the current doc version to check for consistency', function () {
-          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.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 () {
-          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 () {
-          this.multi.rpush
-            .calledWith(
-              `DocOps:${this.docId}`,
-              JSON.stringify(this.ops[0]),
-              JSON.stringify(this.ops[1])
-            )
-            .should.equal(true)
-        })
-
-        it('should renew the expiry ttl on the doc ops array', function () {
-          this.multi.expire
-            .calledWith(`DocOps:${this.docId}`, this.RedisManager.DOC_OPS_TTL)
-            .should.equal(true)
-        })
+      beforeEach(function () {
+        this.RedisManager.getDocVersion
+          .withArgs(this.docId)
+          .yields(null, this.version - this.ops.length)
+        this.RedisManager.updateDocument(
+          this.project_id,
+          this.docId,
+          this.lines,
+          this.version,
+          this.ops,
+          this.ranges,
+          this.updateMeta,
+          this.callback
+        )
+      })
 
-        it('should truncate the list to 100 members', function () {
-          this.multi.ltrim
-            .calledWith(
-              `DocOps:${this.docId}`,
-              -this.RedisManager.DOC_OPS_MAX_LENGTH,
-              -1
-            )
-            .should.equal(true)
-        })
+      it('should get the current doc version to check for consistency', function () {
+        this.RedisManager.getDocVersion
+          .calledWith(this.docId)
+          .should.equal(true)
+      })
 
-        it('should push the updates into the history ops list', function () {
-          this.multi.rpush
-            .calledWith(
-              `UncompressedHistoryOps:${this.docId}`,
-              JSON.stringify(this.ops[0]),
-              JSON.stringify(this.ops[1])
-            )
-            .should.equal(true)
-        })
+      it('should set most details in a single MSET call', function () {
+        this.multi.mset
+          .calledWith({
+            [`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 push the updates into the project history ops list', function () {
-          this.ProjectHistoryRedisManager.queueOps
-            .calledWith(this.project_id, JSON.stringify(this.ops[0]))
-            .should.equal(true)
-        })
+      it('should set the unflushed time', function () {
+        this.multi.set
+          .calledWith(`UnflushedTime:${this.docId}`, Date.now(), 'NX')
+          .should.equal(true)
+      })
 
-        it('should call the callback', function () {
-          this.callback
-            .calledWith(
-              null,
-              this.doc_update_list_length,
-              this.project_update_list_length
-            )
-            .should.equal(true)
-        })
+      it('should push the doc op into the doc ops list', function () {
+        this.multi.rpush
+          .calledWith(
+            `DocOps:${this.docId}`,
+            JSON.stringify(this.ops[0]),
+            JSON.stringify(this.ops[1])
+          )
+          .should.equal(true)
+      })
 
-        it('should not log any errors', function () {
-          this.logger.error.calledWith().should.equal(false)
-        })
+      it('should renew the expiry ttl on the doc ops array', function () {
+        this.multi.expire
+          .calledWith(`DocOps:${this.docId}`, this.RedisManager.DOC_OPS_TTL)
+          .should.equal(true)
       })
 
-      describe('with project history disabled', function () {
-        beforeEach(function () {
-          this.settings.apis.project_history.enabled = false
-          this.RedisManager.getDocVersion
-            .withArgs(this.docId)
-            .yields(null, this.version - this.ops.length)
-          this.RedisManager.updateDocument(
-            this.project_id,
-            this.docId,
-            this.lines,
-            this.version,
-            this.ops,
-            this.ranges,
-            this.updateMeta,
-            this.callback
+      it('should truncate the list to 100 members', function () {
+        this.multi.ltrim
+          .calledWith(
+            `DocOps:${this.docId}`,
+            -this.RedisManager.DOC_OPS_MAX_LENGTH,
+            -1
           )
-        })
+          .should.equal(true)
+      })
 
-        it('should not push the updates into the project history ops list', function () {
-          this.ProjectHistoryRedisManager.queueOps.called.should.equal(false)
-        })
+      it('should push the updates into the project history ops list', function () {
+        this.ProjectHistoryRedisManager.queueOps
+          .calledWith(this.project_id, JSON.stringify(this.ops[0]))
+          .should.equal(true)
+      })
 
-        it('should call the callback', function () {
-          this.callback
-            .calledWith(null, this.doc_update_list_length)
-            .should.equal(true)
-        })
+      it('should call the callback', function () {
+        this.callback
+          .calledWith(null, this.project_update_list_length)
+          .should.equal(true)
+      })
+
+      it('should not log any errors', function () {
+        this.logger.error.calledWith().should.equal(false)
       })
 
       describe('with a doc using project history only', function () {
         beforeEach(function () {
           this.RedisManager.getDocVersion
             .withArgs(this.docId)
-            .yields(null, this.version - this.ops.length, 'project-history')
+            .yields(null, this.version - this.ops.length)
           this.RedisManager.updateDocument(
             this.project_id,
             this.docId,
@@ -611,12 +547,6 @@ describe('RedisManager', function () {
           )
         })
 
-        it('should not push the updates to the track-changes ops list', function () {
-          this.multi.rpush
-            .calledWith(`UncompressedHistoryOps:${this.docId}`)
-            .should.equal(false)
-        })
-
         it('should push the updates into the project history ops list', function () {
           this.ProjectHistoryRedisManager.queueOps
             .calledWith(this.project_id, JSON.stringify(this.ops[0]))
@@ -625,7 +555,7 @@ describe('RedisManager', function () {
 
         it('should call the callback with the project update count only', function () {
           this.callback
-            .calledWith(null, undefined, this.project_update_list_length)
+            .calledWith(null, this.project_update_list_length)
             .should.equal(true)
         })
       })
@@ -993,7 +923,6 @@ describe('RedisManager', function () {
           `Ranges:${this.docId}`,
           `Pathname:${this.docId}`,
           `ProjectHistoryId:${this.docId}`,
-          `ProjectHistoryType:${this.docId}`,
           `UnflushedTime:${this.docId}`,
           `lastUpdatedAt:${this.docId}`,
           `lastUpdatedBy:${this.docId}`
@@ -1083,44 +1012,15 @@ describe('RedisManager', function () {
     describe('getDocVersion', function () {
       beforeEach(function () {
         this.version = 12345
+        this.rclient.mget = sinon
+          .stub()
+          .withArgs(`DocVersion:${this.docId}`)
+          .callsArgWith(1, null, [`${this.version}`])
+        this.RedisManager.getDocVersion(this.docId, this.callback)
       })
 
-      describe('when the document does not have a project history type set', function () {
-        beforeEach(function () {
-          this.rclient.mget = sinon
-            .stub()
-            .withArgs(
-              `DocVersion:${this.docId}`,
-              `ProjectHistoryType:${this.docId}`
-            )
-            .callsArgWith(2, null, [`${this.version}`])
-          this.RedisManager.getDocVersion(this.docId, this.callback)
-        })
-
-        it('should return the document version and an undefined history type', function () {
-          this.callback
-            .calledWithExactly(null, this.version, undefined)
-            .should.equal(true)
-        })
-      })
-
-      describe('when the document has a project history type set', function () {
-        beforeEach(function () {
-          this.rclient.mget = sinon
-            .stub()
-            .withArgs(
-              `DocVersion:${this.docId}`,
-              `ProjectHistoryType:${this.docId}`
-            )
-            .callsArgWith(2, null, [`${this.version}`, 'project-history'])
-          this.RedisManager.getDocVersion(this.docId, this.callback)
-        })
-
-        it('should return the document version and history type', function () {
-          this.callback
-            .calledWithExactly(null, this.version, 'project-history')
-            .should.equal(true)
-        })
+      it('should return the document version', function () {
+        this.callback.calledWithExactly(null, this.version).should.equal(true)
       })
     })
   })

services/document-updater/test/unit/js/UpdateManager/UpdateManagerTests.js

@@ -1,5 +1,4 @@
 /* eslint-disable
-    no-return-assign,
     no-unused-vars,
 */
 // TODO: This file was created by bulk-decaffeinate.
@@ -7,7 +6,6 @@
 /*
  * 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
  * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
  */
@@ -22,7 +20,7 @@ describe('UpdateManager', function () {
     this.projectHistoryId = 'history-id-123'
     this.doc_id = 'document-id-123'
     this.callback = sinon.stub()
-    return (this.UpdateManager = SandboxedModule.require(modulePath, {
+    this.UpdateManager = SandboxedModule.require(modulePath, {
       requires: {
         './LockManager': (this.LockManager = {}),
         './RedisManager': (this.RedisManager = {}),
@@ -56,13 +54,13 @@ describe('UpdateManager', function () {
           return Profiler
         })()),
       },
-    }))
+    })
   })
 
   describe('processOutstandingUpdates', function () {
     beforeEach(function () {
       this.UpdateManager.fetchAndApplyUpdates = sinon.stub().callsArg(2)
-      return this.UpdateManager.processOutstandingUpdates(
+      this.UpdateManager.processOutstandingUpdates(
         this.project_id,
         this.doc_id,
         this.callback
@@ -70,17 +68,17 @@ describe('UpdateManager', function () {
     })
 
     it('should apply the updates', function () {
-      return this.UpdateManager.fetchAndApplyUpdates
+      this.UpdateManager.fetchAndApplyUpdates
         .calledWith(this.project_id, this.doc_id)
         .should.equal(true)
     })
 
     it('should call the callback', function () {
-      return this.callback.called.should.equal(true)
+      this.callback.called.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)
     })
   })
 
@@ -94,14 +92,12 @@ describe('UpdateManager', function () {
         this.UpdateManager.continueProcessingUpdatesWithLock = sinon
           .stub()
           .callsArg(2)
-        return (this.UpdateManager.processOutstandingUpdates = sinon
-          .stub()
-          .callsArg(2))
+        this.UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
       })
 
       describe('successfully', function () {
         beforeEach(function () {
-          return this.UpdateManager.processOutstandingUpdatesWithLock(
+          this.UpdateManager.processOutstandingUpdatesWithLock(
             this.project_id,
             this.doc_id,
             this.callback
@@ -109,19 +105,17 @@ describe('UpdateManager', function () {
         })
 
         it('should acquire the lock', function () {
-          return this.LockManager.tryLock
-            .calledWith(this.doc_id)
-            .should.equal(true)
+          this.LockManager.tryLock.calledWith(this.doc_id).should.equal(true)
         })
 
         it('should free the lock', function () {
-          return this.LockManager.releaseLock
+          this.LockManager.releaseLock
             .calledWith(this.doc_id, this.lockValue)
             .should.equal(true)
         })
 
         it('should process the outstanding updates', function () {
-          return this.UpdateManager.processOutstandingUpdates
+          this.UpdateManager.processOutstandingUpdates
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
         })
@@ -130,28 +124,28 @@ describe('UpdateManager', function () {
           this.UpdateManager.processOutstandingUpdates
             .calledAfter(this.LockManager.tryLock)
             .should.equal(true)
-          return this.UpdateManager.processOutstandingUpdates
+          this.UpdateManager.processOutstandingUpdates
             .calledBefore(this.LockManager.releaseLock)
             .should.equal(true)
         })
 
         it('should continue processing new updates that may have come in', function () {
-          return this.UpdateManager.continueProcessingUpdatesWithLock
+          this.UpdateManager.continueProcessingUpdatesWithLock
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
         })
 
-        return it('should return the callback', function () {
-          return this.callback.called.should.equal(true)
+        it('should return the callback', function () {
+          this.callback.called.should.equal(true)
         })
       })
 
-      return describe('when processOutstandingUpdates returns an error', function () {
+      describe('when processOutstandingUpdates returns an error', function () {
         beforeEach(function () {
           this.UpdateManager.processOutstandingUpdates = sinon
             .stub()
             .callsArgWith(2, (this.error = new Error('Something went wrong')))
-          return this.UpdateManager.processOutstandingUpdatesWithLock(
+          this.UpdateManager.processOutstandingUpdatesWithLock(
             this.project_id,
             this.doc_id,
             this.callback
@@ -159,22 +153,22 @@ describe('UpdateManager', function () {
         })
 
         it('should free the lock', function () {
-          return this.LockManager.releaseLock
+          this.LockManager.releaseLock
             .calledWith(this.doc_id, this.lockValue)
             .should.equal(true)
         })
 
-        return it('should return the error in the callback', function () {
-          return this.callback.calledWith(this.error).should.equal(true)
+        it('should return the error in the callback', function () {
+          this.callback.calledWith(this.error).should.equal(true)
         })
       })
     })
 
-    return describe('when the lock is taken', function () {
+    describe('when the lock is taken', function () {
       beforeEach(function () {
         this.LockManager.tryLock = sinon.stub().callsArgWith(1, null, false)
         this.UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
-        return this.UpdateManager.processOutstandingUpdatesWithLock(
+        this.UpdateManager.processOutstandingUpdatesWithLock(
           this.project_id,
           this.doc_id,
           this.callback
@@ -182,13 +176,11 @@ describe('UpdateManager', function () {
       })
 
       it('should return the callback', function () {
-        return this.callback.called.should.equal(true)
+        this.callback.called.should.equal(true)
       })
 
-      return it('should not process the updates', function () {
-        return this.UpdateManager.processOutstandingUpdates.called.should.equal(
-          false
-        )
+      it('should not process the updates', function () {
+        this.UpdateManager.processOutstandingUpdates.called.should.equal(false)
       })
     })
   })
@@ -202,7 +194,7 @@ describe('UpdateManager', function () {
         this.UpdateManager.processOutstandingUpdatesWithLock = sinon
           .stub()
           .callsArg(2)
-        return this.UpdateManager.continueProcessingUpdatesWithLock(
+        this.UpdateManager.continueProcessingUpdatesWithLock(
           this.project_id,
           this.doc_id,
           this.callback
@@ -210,17 +202,17 @@ describe('UpdateManager', function () {
       })
 
       it('should process the outstanding updates', function () {
-        return this.UpdateManager.processOutstandingUpdatesWithLock
+        this.UpdateManager.processOutstandingUpdatesWithLock
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
-      return it('should return the callback', function () {
-        return this.callback.called.should.equal(true)
+      it('should return the callback', function () {
+        this.callback.called.should.equal(true)
       })
     })
 
-    return describe('when there are no outstanding updates', function () {
+    describe('when there are no outstanding updates', function () {
       beforeEach(function () {
         this.RealTimeRedisManager.getUpdatesLength = sinon
           .stub()
@@ -228,7 +220,7 @@ describe('UpdateManager', function () {
         this.UpdateManager.processOutstandingUpdatesWithLock = sinon
           .stub()
           .callsArg(2)
-        return this.UpdateManager.continueProcessingUpdatesWithLock(
+        this.UpdateManager.continueProcessingUpdatesWithLock(
           this.project_id,
           this.doc_id,
           this.callback
@@ -236,13 +228,13 @@ describe('UpdateManager', function () {
       })
 
       it('should not try to process the outstanding updates', function () {
-        return this.UpdateManager.processOutstandingUpdatesWithLock.called.should.equal(
+        this.UpdateManager.processOutstandingUpdatesWithLock.called.should.equal(
           false
         )
       })
 
-      return it('should return the callback', function () {
-        return this.callback.called.should.equal(true)
+      it('should return the callback', function () {
+        this.callback.called.should.equal(true)
       })
     })
   })
@@ -259,7 +251,7 @@ describe('UpdateManager', function () {
         this.UpdateManager.applyUpdate = sinon
           .stub()
           .callsArgWith(3, null, this.updatedDocLines, this.version)
-        return this.UpdateManager.fetchAndApplyUpdates(
+        this.UpdateManager.fetchAndApplyUpdates(
           this.project_id,
           this.doc_id,
           this.callback
@@ -267,25 +259,25 @@ describe('UpdateManager', function () {
       })
 
       it('should get the pending updates', function () {
-        return this.RealTimeRedisManager.getPendingUpdatesForDoc
+        this.RealTimeRedisManager.getPendingUpdatesForDoc
           .calledWith(this.doc_id)
           .should.equal(true)
       })
 
       it('should apply the updates', function () {
-        return Array.from(this.updates).map(update =>
+        Array.from(this.updates).map(update =>
           this.UpdateManager.applyUpdate
             .calledWith(this.project_id, this.doc_id, update)
             .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 there are no updates', function () {
+    describe('when there are no updates', function () {
       beforeEach(function () {
         this.updates = []
         this.RealTimeRedisManager.getPendingUpdatesForDoc = sinon
@@ -293,7 +285,7 @@ describe('UpdateManager', function () {
           .callsArgWith(1, null, this.updates)
         this.UpdateManager.applyUpdate = sinon.stub()
         this.RedisManager.setDocument = sinon.stub()
-        return this.UpdateManager.fetchAndApplyUpdates(
+        this.UpdateManager.fetchAndApplyUpdates(
           this.project_id,
           this.doc_id,
           this.callback
@@ -301,11 +293,11 @@ describe('UpdateManager', function () {
       })
 
       it('should not call applyUpdate', function () {
-        return this.UpdateManager.applyUpdate.called.should.equal(false)
+        this.UpdateManager.applyUpdate.called.should.equal(false)
       })
 
-      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)
       })
     })
   })
@@ -323,7 +315,6 @@ describe('UpdateManager', function () {
         { v: 42, op: 'mock-op-42' },
         { v: 45, op: 'mock-op-45' },
       ]
-      this.doc_ops_length = sinon.stub()
       this.project_ops_length = sinon.stub()
       this.pathname = '/a/b/c.tex'
       this.DocumentManager.getDoc = sinon
@@ -344,17 +335,15 @@ describe('UpdateManager', function () {
         .yields(null, this.updatedDocLines, this.version, this.appliedOps)
       this.RedisManager.updateDocument = sinon
         .stub()
-        .yields(null, this.doc_ops_length, this.project_ops_length)
+        .yields(null, this.project_ops_length)
       this.RealTimeRedisManager.sendData = sinon.stub()
       this.UpdateManager._addProjectHistoryMetadataToOps = sinon.stub()
-      return (this.HistoryManager.recordAndFlushHistoryOps = sinon
-        .stub()
-        .callsArg(5))
+      this.HistoryManager.recordAndFlushHistoryOps = sinon.stub()
     })
 
     describe('normally', function () {
       beforeEach(function () {
-        return this.UpdateManager.applyUpdate(
+        this.UpdateManager.applyUpdate(
           this.project_id,
           this.doc_id,
           this.update,
@@ -363,7 +352,7 @@ describe('UpdateManager', function () {
       })
 
       it('should apply the updates via ShareJS', function () {
-        return this.ShareJsUpdateManager.applyUpdate
+        this.ShareJsUpdateManager.applyUpdate
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -375,7 +364,7 @@ describe('UpdateManager', function () {
       })
 
       it('should update the ranges', function () {
-        return this.RangesManager.applyUpdate
+        this.RangesManager.applyUpdate
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -387,7 +376,7 @@ describe('UpdateManager', function () {
       })
 
       it('should save the document', function () {
-        return this.RedisManager.updateDocument
+        this.RedisManager.updateDocument
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -401,7 +390,7 @@ describe('UpdateManager', function () {
       })
 
       it('should add metadata to the ops', function () {
-        return this.UpdateManager._addProjectHistoryMetadataToOps
+        this.UpdateManager._addProjectHistoryMetadataToOps
           .calledWith(
             this.appliedOps,
             this.pathname,
@@ -412,26 +401,20 @@ describe('UpdateManager', function () {
       })
 
       it('should push the applied ops into the history queue', function () {
-        return this.HistoryManager.recordAndFlushHistoryOps
-          .calledWith(
-            this.project_id,
-            this.doc_id,
-            this.appliedOps,
-            this.doc_ops_length,
-            this.project_ops_length
-          )
+        this.HistoryManager.recordAndFlushHistoryOps
+          .calledWith(this.project_id, this.appliedOps, this.project_ops_length)
           .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('with UTF-16 surrogate pairs in the update', function () {
       beforeEach(function () {
         this.update = { op: [{ p: 42, i: '\uD835\uDC00' }] }
-        return this.UpdateManager.applyUpdate(
+        this.UpdateManager.applyUpdate(
           this.project_id,
           this.doc_id,
           this.update,
@@ -439,13 +422,13 @@ describe('UpdateManager', function () {
         )
       })
 
-      return it('should apply the update but with surrogate pairs removed', function () {
+      it('should apply the update but with surrogate pairs removed', function () {
         this.ShareJsUpdateManager.applyUpdate
           .calledWith(this.project_id, this.doc_id, this.update)
           .should.equal(true)
 
         // \uFFFD is 'replacement character'
-        return this.update.op[0].i.should.equal('\uFFFD\uFFFD')
+        this.update.op[0].i.should.equal('\uFFFD\uFFFD')
       })
     })
 
@@ -453,7 +436,7 @@ describe('UpdateManager', function () {
       beforeEach(function () {
         this.error = new Error('something went wrong')
         this.ShareJsUpdateManager.applyUpdate = sinon.stub().yields(this.error)
-        return this.UpdateManager.applyUpdate(
+        this.UpdateManager.applyUpdate(
           this.project_id,
           this.doc_id,
           this.update,
@@ -462,7 +445,7 @@ describe('UpdateManager', function () {
       })
 
       it('should call RealTimeRedisManager.sendData with the error', function () {
-        return this.RealTimeRedisManager.sendData
+        this.RealTimeRedisManager.sendData
           .calledWith({
             project_id: this.project_id,
             doc_id: this.doc_id,
@@ -471,18 +454,18 @@ describe('UpdateManager', function () {
           .should.equal(true)
       })
 
-      return it('should call the callback with the error', function () {
-        return this.callback.calledWith(this.error).should.equal(true)
+      it('should call the callback with the error', function () {
+        this.callback.calledWith(this.error).should.equal(true)
       })
     })
 
-    return describe('when ranges get collapsed', function () {
+    describe('when ranges get collapsed', function () {
       beforeEach(function () {
         this.RangesManager.applyUpdate = sinon
           .stub()
           .yields(null, this.updated_ranges, true)
         this.SnapshotManager.recordSnapshot = sinon.stub().yields()
-        return this.UpdateManager.applyUpdate(
+        this.UpdateManager.applyUpdate(
           this.project_id,
           this.doc_id,
           this.update,
@@ -494,8 +477,8 @@ describe('UpdateManager', function () {
         this.Metrics.inc.calledWith('doc-snapshot').should.equal(true)
       })
 
-      return it('should call SnapshotManager.recordSnapshot', function () {
-        return this.SnapshotManager.recordSnapshot
+      it('should call SnapshotManager.recordSnapshot', function () {
+        this.SnapshotManager.recordSnapshot
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -510,7 +493,7 @@ describe('UpdateManager', function () {
   })
 
   describe('_addProjectHistoryMetadataToOps', function () {
-    return it('should add projectHistoryId, pathname and doc_length metadata to the ops', function () {
+    it('should add projectHistoryId, pathname and doc_length metadata to the ops', function () {
       const lines = ['some', 'test', 'data']
       const appliedOps = [
         {
@@ -535,7 +518,7 @@ describe('UpdateManager', function () {
         this.projectHistoryId,
         lines
       )
-      return appliedOps.should.deep.equal([
+      appliedOps.should.deep.equal([
         {
           projectHistoryId: this.projectHistoryId,
           v: 42,
@@ -573,7 +556,7 @@ describe('UpdateManager', function () {
     })
   })
 
-  return describe('lockUpdatesAndDo', function () {
+  describe('lockUpdatesAndDo', function () {
     beforeEach(function () {
       this.method = sinon.stub().callsArgWith(3, null, this.response_arg1)
       this.callback = sinon.stub()
@@ -583,14 +566,14 @@ describe('UpdateManager', function () {
       this.LockManager.getLock = sinon
         .stub()
         .callsArgWith(1, null, this.lockValue)
-      return (this.LockManager.releaseLock = sinon.stub().callsArg(2))
+      this.LockManager.releaseLock = sinon.stub().callsArg(2)
     })
 
     describe('successfully', function () {
       beforeEach(function () {
         this.UpdateManager.continueProcessingUpdatesWithLock = sinon.stub()
         this.UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
-        return this.UpdateManager.lockUpdatesAndDo(
+        this.UpdateManager.lockUpdatesAndDo(
           this.method,
           this.project_id,
           this.doc_id,
@@ -600,37 +583,33 @@ describe('UpdateManager', function () {
       })
 
       it('should lock the doc', function () {
-        return this.LockManager.getLock
-          .calledWith(this.doc_id)
-          .should.equal(true)
+        this.LockManager.getLock.calledWith(this.doc_id).should.equal(true)
       })
 
       it('should process any outstanding updates', function () {
-        return this.UpdateManager.processOutstandingUpdates
+        this.UpdateManager.processOutstandingUpdates
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should call the method', function () {
-        return this.method
+        this.method
           .calledWith(this.project_id, this.doc_id, this.arg1)
           .should.equal(true)
       })
 
       it('should return the method response to the callback', function () {
-        return this.callback
-          .calledWith(null, this.response_arg1)
-          .should.equal(true)
+        this.callback.calledWith(null, this.response_arg1).should.equal(true)
       })
 
       it('should release the lock', function () {
-        return this.LockManager.releaseLock
+        this.LockManager.releaseLock
           .calledWith(this.doc_id, this.lockValue)
           .should.equal(true)
       })
 
-      return it('should continue processing updates', function () {
-        return this.UpdateManager.continueProcessingUpdatesWithLock
+      it('should continue processing updates', function () {
+        this.UpdateManager.continueProcessingUpdatesWithLock
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
@@ -641,7 +620,7 @@ describe('UpdateManager', function () {
         this.UpdateManager.processOutstandingUpdates = sinon
           .stub()
           .callsArgWith(2, (this.error = new Error('Something went wrong')))
-        return this.UpdateManager.lockUpdatesAndDo(
+        this.UpdateManager.lockUpdatesAndDo(
           this.method,
           this.project_id,
           this.doc_id,
@@ -651,17 +630,17 @@ describe('UpdateManager', function () {
       })
 
       it('should free the lock', function () {
-        return this.LockManager.releaseLock
+        this.LockManager.releaseLock
           .calledWith(this.doc_id, this.lockValue)
           .should.equal(true)
       })
 
-      return it('should return the error in the callback', function () {
-        return this.callback.calledWith(this.error).should.equal(true)
+      it('should return the error in the callback', function () {
+        this.callback.calledWith(this.error).should.equal(true)
       })
     })
 
-    return describe('when the method returns an error', function () {
+    describe('when the method returns an error', function () {
       beforeEach(function () {
         this.UpdateManager.processOutstandingUpdates = sinon.stub().callsArg(2)
         this.method = sinon
@@ -671,7 +650,7 @@ describe('UpdateManager', function () {
             (this.error = new Error('something went wrong')),
             this.response_arg1
           )
-        return this.UpdateManager.lockUpdatesAndDo(
+        this.UpdateManager.lockUpdatesAndDo(
           this.method,
           this.project_id,
           this.doc_id,
@@ -681,13 +660,13 @@ describe('UpdateManager', function () {
       })
 
       it('should free the lock', function () {
-        return this.LockManager.releaseLock
+        this.LockManager.releaseLock
           .calledWith(this.doc_id, this.lockValue)
           .should.equal(true)
       })
 
-      return it('should return the error in the callback', function () {
-        return this.callback.calledWith(this.error).should.equal(true)
+      it('should return the error in the callback', function () {
+        this.callback.calledWith(this.error).should.equal(true)
       })
     })
   })