Преглед на файлове

Merge pull request #4761 from overleaf/em-peek-doc

Do not unarchive docs when resyncing project history

GitOrigin-RevId: c7df75789c01e6c85b464a9b94b14654d8568407
Eric Mc Sween преди 5 години
родител
ревизия
9f4541c266

+ 1 - 0
services/document-updater/app/js/DocumentManager.js

@@ -600,6 +600,7 @@ module.exports = DocumentManager = {
           return PersistenceManager.getDoc(
             project_id,
             doc_id,
+            { peek: true },
             function (
               error,
               lines,

+ 131 - 171
services/document-updater/app/js/PersistenceManager.js

@@ -1,20 +1,3 @@
-/* eslint-disable
-    camelcase,
-    handle-callback-err,
-    no-unsafe-negation,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS102: Remove unnecessary code created because of implicit returns
- * DS205: Consider reworking code to avoid use of IIFEs
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-let PersistenceManager
 const Settings = require('@overleaf/settings')
 const Errors = require('./Errors')
 const Metrics = require('./Metrics')
@@ -29,172 +12,149 @@ const request = require('requestretry').defaults({
 // hold us up, and need to bail out quickly if there is a problem.
 const MAX_HTTP_REQUEST_LENGTH = 5000 // 5 seconds
 
-const updateMetric = function (method, error, response) {
+function updateMetric(method, error, response) {
   // find the status, with special handling for connection timeouts
   // https://github.com/request/request#timeouts
-  const status = (() => {
-    if ((error != null ? error.connect : undefined) === true) {
-      return `${error.code} (connect)`
-    } else if (error != null) {
-      return error.code
-    } else if (response != null) {
-      return response.statusCode
-    }
-  })()
+  let status
+  if (error && error.connect === true) {
+    status = `${error.code} (connect)`
+  } else if (error) {
+    status = error.code
+  } else if (response) {
+    status = response.statusCode
+  }
+
   Metrics.inc(method, 1, { status })
-  if ((error != null ? error.attempts : undefined) > 1) {
+  if (error && error.attempts > 1) {
     Metrics.inc(`${method}-retries`, 1, { status: 'error' })
   }
-  if ((response != null ? response.attempts : undefined) > 1) {
-    return Metrics.inc(`${method}-retries`, 1, { status: 'success' })
+  if (response && response.attempts > 1) {
+    Metrics.inc(`${method}-retries`, 1, { status: 'success' })
   }
 }
 
-module.exports = PersistenceManager = {
-  getDoc(project_id, doc_id, _callback) {
-    if (_callback == null) {
-      _callback = function (
-        error,
-        lines,
-        version,
-        ranges,
-        pathname,
-        projectHistoryId,
-        projectHistoryType
-      ) {}
-    }
-    const timer = new Metrics.Timer('persistenceManager.getDoc')
-    const callback = function (...args) {
-      timer.done()
-      return _callback(...Array.from(args || []))
-    }
-
-    const urlPath = `/project/${project_id}/doc/${doc_id}`
-    return request(
-      {
-        url: `${Settings.apis.web.url}${urlPath}`,
-        method: 'GET',
-        headers: {
-          accept: 'application/json',
-        },
-        auth: {
-          user: Settings.apis.web.user,
-          pass: Settings.apis.web.pass,
-          sendImmediately: true,
-        },
-        jar: false,
-        timeout: MAX_HTTP_REQUEST_LENGTH,
-      },
-      function (error, res, body) {
-        updateMetric('getDoc', error, res)
-        if (error != null) {
-          logger.error(
-            { err: error, project_id, doc_id },
-            'web API request failed'
-          )
-          return callback(new Error('error connecting to web API'))
-        }
-        if (res.statusCode >= 200 && res.statusCode < 300) {
-          try {
-            body = JSON.parse(body)
-          } catch (e) {
-            return callback(e)
-          }
-          if (body.lines == null) {
-            return callback(new Error('web API response had no doc lines'))
-          }
-          if (body.version == null || !body.version instanceof Number) {
-            return callback(
-              new Error('web API response had no valid doc version')
-            )
-          }
-          if (body.pathname == null) {
-            return callback(
-              new Error('web API response had no valid doc pathname')
-            )
-          }
-          return callback(
-            null,
-            body.lines,
-            body.version,
-            body.ranges,
-            body.pathname,
-            body.projectHistoryId,
-            body.projectHistoryType
-          )
-        } else if (res.statusCode === 404) {
-          return callback(
-            new Errors.NotFoundError(`doc not not found: ${urlPath}`)
-          )
-        } else {
-          return callback(
-            new Error(`error accessing web API: ${urlPath} ${res.statusCode}`)
-          )
-        }
-      }
-    )
-  },
+function getDoc(projectId, docId, options = {}, _callback) {
+  const timer = new Metrics.Timer('persistenceManager.getDoc')
+  if (typeof options === 'function') {
+    _callback = options
+    options = {}
+  }
+  const callback = function (...args) {
+    timer.done()
+    _callback(...args)
+  }
 
-  setDoc(
-    project_id,
-    doc_id,
-    lines,
-    version,
-    ranges,
-    lastUpdatedAt,
-    lastUpdatedBy,
-    _callback
-  ) {
-    if (_callback == null) {
-      _callback = function (error) {}
+  const urlPath = `/project/${projectId}/doc/${docId}`
+  const requestParams = {
+    url: `${Settings.apis.web.url}${urlPath}`,
+    method: 'GET',
+    headers: {
+      accept: 'application/json',
+    },
+    auth: {
+      user: Settings.apis.web.user,
+      pass: Settings.apis.web.pass,
+      sendImmediately: true,
+    },
+    jar: false,
+    timeout: MAX_HTTP_REQUEST_LENGTH,
+  }
+  if (options.peek) {
+    requestParams.qs = { peek: 'true' }
+  }
+  request(requestParams, (error, res, body) => {
+    updateMetric('getDoc', error, res)
+    if (error) {
+      logger.error({ err: error, projectId, docId }, 'web API request failed')
+      return callback(new Error('error connecting to web API'))
     }
-    const timer = new Metrics.Timer('persistenceManager.setDoc')
-    const callback = function (...args) {
-      timer.done()
-      return _callback(...Array.from(args || []))
+    if (res.statusCode >= 200 && res.statusCode < 300) {
+      try {
+        body = JSON.parse(body)
+      } catch (e) {
+        return callback(e)
+      }
+      if (body.lines == null) {
+        return callback(new Error('web API response had no doc lines'))
+      }
+      if (body.version == null) {
+        return callback(new Error('web API response had no valid doc version'))
+      }
+      if (body.pathname == null) {
+        return callback(new Error('web API response had no valid doc pathname'))
+      }
+      callback(
+        null,
+        body.lines,
+        body.version,
+        body.ranges,
+        body.pathname,
+        body.projectHistoryId,
+        body.projectHistoryType
+      )
+    } else if (res.statusCode === 404) {
+      callback(new Errors.NotFoundError(`doc not not found: ${urlPath}`))
+    } else {
+      callback(
+        new Error(`error accessing web API: ${urlPath} ${res.statusCode}`)
+      )
     }
+  })
+}
+
+function setDoc(
+  projectId,
+  docId,
+  lines,
+  version,
+  ranges,
+  lastUpdatedAt,
+  lastUpdatedBy,
+  _callback
+) {
+  const timer = new Metrics.Timer('persistenceManager.setDoc')
+  const callback = function (...args) {
+    timer.done()
+    _callback(...args)
+  }
 
-    const urlPath = `/project/${project_id}/doc/${doc_id}`
-    return request(
-      {
-        url: `${Settings.apis.web.url}${urlPath}`,
-        method: 'POST',
-        json: {
-          lines,
-          ranges,
-          version,
-          lastUpdatedBy,
-          lastUpdatedAt,
-        },
-        auth: {
-          user: Settings.apis.web.user,
-          pass: Settings.apis.web.pass,
-          sendImmediately: true,
-        },
-        jar: false,
-        timeout: MAX_HTTP_REQUEST_LENGTH,
+  const urlPath = `/project/${projectId}/doc/${docId}`
+  request(
+    {
+      url: `${Settings.apis.web.url}${urlPath}`,
+      method: 'POST',
+      json: {
+        lines,
+        ranges,
+        version,
+        lastUpdatedBy,
+        lastUpdatedAt,
+      },
+      auth: {
+        user: Settings.apis.web.user,
+        pass: Settings.apis.web.pass,
+        sendImmediately: true,
       },
-      function (error, res, body) {
-        updateMetric('setDoc', error, res)
-        if (error != null) {
-          logger.error(
-            { err: error, project_id, doc_id },
-            'web API request failed'
-          )
-          return callback(new Error('error connecting to web API'))
-        }
-        if (res.statusCode >= 200 && res.statusCode < 300) {
-          return callback(null)
-        } else if (res.statusCode === 404) {
-          return callback(
-            new Errors.NotFoundError(`doc not not found: ${urlPath}`)
-          )
-        } else {
-          return callback(
-            new Error(`error accessing web API: ${urlPath} ${res.statusCode}`)
-          )
-        }
+      jar: false,
+      timeout: MAX_HTTP_REQUEST_LENGTH,
+    },
+    (error, res, body) => {
+      updateMetric('setDoc', error, res)
+      if (error) {
+        logger.error({ err: error, projectId, docId }, 'web API request failed')
+        return callback(new Error('error connecting to web API'))
       }
-    )
-  },
+      if (res.statusCode >= 200 && res.statusCode < 300) {
+        callback(null)
+      } else if (res.statusCode === 404) {
+        callback(new Errors.NotFoundError(`doc not not found: ${urlPath}`))
+      } else {
+        callback(
+          new Error(`error accessing web API: ${urlPath} ${res.statusCode}`)
+        )
+      }
+    }
+  )
 }
+
+module.exports = { getDoc, setDoc }

+ 150 - 187
services/document-updater/test/unit/js/DocumentManager/DocumentManagerTests.js

@@ -1,17 +1,3 @@
-/* eslint-disable
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS101: Remove unnecessary use of Array.from
- * DS102: Remove unnecessary code created because of implicit returns
- * DS206: Consider reworking classes to avoid initClass
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const sinon = require('sinon')
 const modulePath = '../../../../app/js/DocumentManager.js'
 const SandboxedModule = require('sandboxed-module')
@@ -20,8 +6,12 @@ const tk = require('timekeeper')
 
 describe('DocumentManager', function () {
   beforeEach(function () {
-    let Timer
     tk.freeze(new Date())
+    this.Metrics = {
+      Timer: class Timer {},
+    }
+    this.Metrics.Timer.prototype.done = sinon.stub()
+
     this.DocumentManager = SandboxedModule.require(modulePath, {
       requires: {
         './RedisManager': (this.RedisManager = {}),
@@ -31,17 +21,7 @@ describe('DocumentManager', function () {
           flushDocChangesAsync: sinon.stub(),
           flushProjectChangesAsync: sinon.stub(),
         }),
-        './Metrics': (this.Metrics = {
-          Timer: (Timer = (function () {
-            Timer = class Timer {
-              static initClass() {
-                this.prototype.done = sinon.stub()
-              }
-            }
-            Timer.initClass()
-            return Timer
-          })()),
-        }),
+        './Metrics': this.Metrics,
         './RealTimeRedisManager': (this.RealTimeRedisManager = {}),
         './DiffCodec': (this.DiffCodec = {}),
         './UpdateManager': (this.UpdateManager = {}),
@@ -61,11 +41,11 @@ describe('DocumentManager', function () {
     this.pathname = '/a/b/c.tex'
     this.unflushedTime = Date.now()
     this.lastUpdatedAt = Date.now()
-    return (this.lastUpdatedBy = 'last-author-id')
+    this.lastUpdatedBy = 'last-author-id'
   })
 
   afterEach(function () {
-    return tk.reset()
+    tk.reset()
   })
 
   describe('flushAndDeleteDoc', function () {
@@ -73,7 +53,7 @@ describe('DocumentManager', function () {
       beforeEach(function () {
         this.RedisManager.removeDocFromMemory = sinon.stub().callsArg(2)
         this.DocumentManager.flushDocIfLoaded = sinon.stub().callsArgWith(2)
-        return this.DocumentManager.flushAndDeleteDoc(
+        this.DocumentManager.flushAndDeleteDoc(
           this.project_id,
           this.doc_id,
           {},
@@ -82,67 +62,65 @@ describe('DocumentManager', function () {
       })
 
       it('should flush the doc', function () {
-        return this.DocumentManager.flushDocIfLoaded
+        this.DocumentManager.flushDocIfLoaded
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should remove the doc from redis', function () {
-        return this.RedisManager.removeDocFromMemory
+        this.RedisManager.removeDocFromMemory
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should call the callback without error', function () {
-        return this.callback.calledWith(null).should.equal(true)
+        this.callback.calledWith(null).should.equal(true)
       })
 
       it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
 
-      return it('should flush to the history api', function () {
-        return this.HistoryManager.flushDocChangesAsync
+      it('should flush to the history api', function () {
+        this.HistoryManager.flushDocChangesAsync
           .calledWithExactly(this.project_id, this.doc_id)
           .should.equal(true)
       })
     })
 
-    return describe('when a flush error occurs', function () {
+    describe('when a flush error occurs', function () {
       beforeEach(function () {
         this.DocumentManager.flushDocIfLoaded = sinon
           .stub()
           .callsArgWith(2, new Error('boom!'))
-        return (this.RedisManager.removeDocFromMemory = sinon
-          .stub()
-          .callsArg(2))
+        this.RedisManager.removeDocFromMemory = sinon.stub().callsArg(2)
       })
 
       it('should not remove the doc from redis', function (done) {
-        return this.DocumentManager.flushAndDeleteDoc(
+        this.DocumentManager.flushAndDeleteDoc(
           this.project_id,
           this.doc_id,
           {},
           error => {
             error.should.exist
             this.RedisManager.removeDocFromMemory.called.should.equal(false)
-            return done()
+            done()
           }
         )
       })
 
-      return describe('when ignoring flush errors', function () {
-        return it('should remove the doc from redis', function (done) {
-          return this.DocumentManager.flushAndDeleteDoc(
+      describe('when ignoring flush errors', function () {
+        it('should remove the doc from redis', function (done) {
+          this.DocumentManager.flushAndDeleteDoc(
             this.project_id,
             this.doc_id,
             { ignoreFlushErrors: true },
             error => {
-              if (error != null) {
+              if (error) {
                 return done(error)
               }
               this.RedisManager.removeDocFromMemory.called.should.equal(true)
-              return done()
+              done()
             }
           )
         })
@@ -171,7 +149,7 @@ describe('DocumentManager', function () {
           .stub()
           .callsArgWith(1, null)
         this.PersistenceManager.setDoc = sinon.stub().yields()
-        return this.DocumentManager.flushDocIfLoaded(
+        this.DocumentManager.flushDocIfLoaded(
           this.project_id,
           this.doc_id,
           this.callback
@@ -179,13 +157,13 @@ describe('DocumentManager', function () {
       })
 
       it('should get the doc from redis', function () {
-        return this.RedisManager.getDoc
+        this.RedisManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should write the doc lines to the persistence layer', function () {
-        return this.PersistenceManager.setDoc
+        this.PersistenceManager.setDoc
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -199,21 +177,21 @@ describe('DocumentManager', function () {
       })
 
       it('should call the callback without error', function () {
-        return this.callback.calledWith(null).should.equal(true)
+        this.callback.calledWith(null).should.equal(true)
       })
 
-      return it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+      it('should time the execution', function () {
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
     })
 
-    return describe('when the document is not in Redis', function () {
+    describe('when the document is not in Redis', function () {
       beforeEach(function () {
         this.RedisManager.getDoc = sinon
           .stub()
           .callsArgWith(2, null, null, null, null)
         this.PersistenceManager.setDoc = sinon.stub().yields()
-        return this.DocumentManager.flushDocIfLoaded(
+        this.DocumentManager.flushDocIfLoaded(
           this.project_id,
           this.doc_id,
           this.callback
@@ -221,7 +199,7 @@ describe('DocumentManager', function () {
       })
 
       it('should get the doc from redis', function () {
-        return this.RedisManager.getDoc
+        this.RedisManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
@@ -231,11 +209,11 @@ describe('DocumentManager', function () {
       })
 
       it('should call the callback without error', function () {
-        return this.callback.calledWith(null).should.equal(true)
+        this.callback.calledWith(null).should.equal(true)
       })
 
-      return it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+      it('should time the execution', function () {
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
     })
   })
@@ -257,7 +235,7 @@ describe('DocumentManager', function () {
         this.RedisManager.getPreviousDocOps = sinon
           .stub()
           .callsArgWith(3, null, this.ops)
-        return this.DocumentManager.getDocAndRecentOps(
+        this.DocumentManager.getDocAndRecentOps(
           this.project_id,
           this.doc_id,
           this.fromVersion,
@@ -266,19 +244,19 @@ describe('DocumentManager', function () {
       })
 
       it('should get the doc', function () {
-        return this.DocumentManager.getDoc
+        this.DocumentManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should get the doc ops', function () {
-        return this.RedisManager.getPreviousDocOps
+        this.RedisManager.getPreviousDocOps
           .calledWith(this.doc_id, this.fromVersion, this.version)
           .should.equal(true)
       })
 
       it('should call the callback with the doc info', function () {
-        return this.callback
+        this.callback
           .calledWith(
             null,
             this.lines,
@@ -291,12 +269,12 @@ describe('DocumentManager', function () {
           .should.equal(true)
       })
 
-      return it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+      it('should time the execution', function () {
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
     })
 
-    return describe('with no previous version specified', function () {
+    describe('with no previous version specified', function () {
       beforeEach(function () {
         this.DocumentManager.getDoc = sinon
           .stub()
@@ -312,7 +290,7 @@ describe('DocumentManager', function () {
         this.RedisManager.getPreviousDocOps = sinon
           .stub()
           .callsArgWith(3, null, this.ops)
-        return this.DocumentManager.getDocAndRecentOps(
+        this.DocumentManager.getDocAndRecentOps(
           this.project_id,
           this.doc_id,
           -1,
@@ -321,17 +299,17 @@ describe('DocumentManager', function () {
       })
 
       it('should get the doc', function () {
-        return this.DocumentManager.getDoc
+        this.DocumentManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should not need to get the doc ops', function () {
-        return this.RedisManager.getPreviousDocOps.called.should.equal(false)
+        this.RedisManager.getPreviousDocOps.called.should.equal(false)
       })
 
       it('should call the callback with the doc info', function () {
-        return this.callback
+        this.callback
           .calledWith(
             null,
             this.lines,
@@ -344,8 +322,8 @@ describe('DocumentManager', function () {
           .should.equal(true)
       })
 
-      return it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+      it('should time the execution', function () {
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
     })
   })
@@ -365,21 +343,17 @@ describe('DocumentManager', function () {
             this.projectHistoryId,
             this.unflushedTime
           )
-        return this.DocumentManager.getDoc(
-          this.project_id,
-          this.doc_id,
-          this.callback
-        )
+        this.DocumentManager.getDoc(this.project_id, this.doc_id, this.callback)
       })
 
       it('should get the doc from Redis', function () {
-        return this.RedisManager.getDoc
+        this.RedisManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should call the callback with the doc info', function () {
-        return this.callback
+        this.callback
           .calledWith(
             null,
             this.lines,
@@ -393,12 +367,12 @@ describe('DocumentManager', function () {
           .should.equal(true)
       })
 
-      return it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+      it('should time the execution', function () {
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
     })
 
-    return describe('when the doc does not exist in Redis', function () {
+    describe('when the doc does not exist in Redis', function () {
       beforeEach(function () {
         this.RedisManager.getDoc = sinon
           .stub()
@@ -417,27 +391,23 @@ describe('DocumentManager', function () {
           )
         this.RedisManager.putDocInMemory = sinon.stub().yields()
         this.RedisManager.setHistoryType = sinon.stub().yields()
-        return this.DocumentManager.getDoc(
-          this.project_id,
-          this.doc_id,
-          this.callback
-        )
+        this.DocumentManager.getDoc(this.project_id, this.doc_id, this.callback)
       })
 
       it('should try to get the doc from Redis', function () {
-        return this.RedisManager.getDoc
+        this.RedisManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should get the doc from the PersistenceManager', function () {
-        return this.PersistenceManager.getDoc
+        this.PersistenceManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should set the doc in Redis', function () {
-        return this.RedisManager.putDocInMemory
+        this.RedisManager.putDocInMemory
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -451,13 +421,13 @@ describe('DocumentManager', function () {
       })
 
       it('should set the history type in Redis', function () {
-        return this.RedisManager.setHistoryType
+        this.RedisManager.setHistoryType
           .calledWith(this.doc_id, this.projectHistoryType)
           .should.equal(true)
       })
 
       it('should call the callback with the doc info', function () {
-        return this.callback
+        this.callback
           .calledWith(
             null,
             this.lines,
@@ -471,14 +441,14 @@ describe('DocumentManager', function () {
           .should.equal(true)
       })
 
-      return it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+      it('should time the execution', function () {
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
     })
   })
 
   describe('setDoc', function () {
-    return describe('with plain tex lines', function () {
+    describe('with plain tex lines', function () {
       beforeEach(function () {
         this.beforeLines = ['before', 'lines']
         this.afterLines = ['after', 'lines']
@@ -504,14 +474,12 @@ describe('DocumentManager', function () {
           .callsArgWith(2, null, this.ops)
         this.UpdateManager.applyUpdate = sinon.stub().callsArgWith(3, null)
         this.DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2)
-        return (this.DocumentManager.flushAndDeleteDoc = sinon
-          .stub()
-          .callsArg(3))
+        this.DocumentManager.flushAndDeleteDoc = sinon.stub().callsArg(3)
       })
 
       describe('when already loaded', function () {
         beforeEach(function () {
-          return this.DocumentManager.setDoc(
+          this.DocumentManager.setDoc(
             this.project_id,
             this.doc_id,
             this.afterLines,
@@ -523,19 +491,19 @@ describe('DocumentManager', function () {
         })
 
         it('should get the current doc lines', function () {
-          return this.DocumentManager.getDoc
+          this.DocumentManager.getDoc
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
         })
 
         it('should return a diff of the old and new lines', function () {
-          return this.DiffCodec.diffAsShareJsOp
+          this.DiffCodec.diffAsShareJsOp
             .calledWith(this.beforeLines, this.afterLines)
             .should.equal(true)
         })
 
         it('should apply the diff as a ShareJS op', function () {
-          return this.UpdateManager.applyUpdate
+          this.UpdateManager.applyUpdate
             .calledWith(this.project_id, this.doc_id, {
               doc: this.doc_id,
               v: this.version,
@@ -550,23 +518,23 @@ describe('DocumentManager', function () {
         })
 
         it('should flush the doc to Mongo', function () {
-          return this.DocumentManager.flushDocIfLoaded
+          this.DocumentManager.flushDocIfLoaded
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
         })
 
         it('should not flush the project history', function () {
-          return this.HistoryManager.flushProjectChangesAsync.called.should.equal(
+          this.HistoryManager.flushProjectChangesAsync.called.should.equal(
             false
           )
         })
 
         it('should call the callback', function () {
-          return this.callback.calledWith(null).should.equal(true)
+          this.callback.calledWith(null).should.equal(true)
         })
 
-        return it('should time the execution', function () {
-          return this.Metrics.Timer.prototype.done.called.should.equal(true)
+        it('should time the execution', function () {
+          this.Metrics.Timer.prototype.done.called.should.equal(true)
         })
       })
 
@@ -583,7 +551,7 @@ describe('DocumentManager', function () {
               null,
               false
             )
-          return this.DocumentManager.setDoc(
+          this.DocumentManager.setDoc(
             this.project_id,
             this.doc_id,
             this.afterLines,
@@ -595,13 +563,13 @@ describe('DocumentManager', function () {
         })
 
         it('should flush and delete the doc from the doc updater', function () {
-          return this.DocumentManager.flushAndDeleteDoc
+          this.DocumentManager.flushAndDeleteDoc
             .calledWith(this.project_id, this.doc_id, {})
             .should.equal(true)
         })
 
-        return it('should not flush the project history', function () {
-          return this.HistoryManager.flushProjectChangesAsync
+        it('should not flush the project history', function () {
+          this.HistoryManager.flushProjectChangesAsync
             .calledWithExactly(this.project_id)
             .should.equal(true)
         })
@@ -609,7 +577,7 @@ describe('DocumentManager', function () {
 
       describe('without new lines', function () {
         beforeEach(function () {
-          return this.DocumentManager.setDoc(
+          this.DocumentManager.setDoc(
             this.project_id,
             this.doc_id,
             null,
@@ -621,17 +589,15 @@ describe('DocumentManager', function () {
         })
 
         it('should return the callback with an error', function () {
-          return this.callback.calledWith(
-            new Error('No lines were passed to setDoc')
-          )
+          this.callback.calledWith(new Error('No lines were passed to setDoc'))
         })
 
-        return it('should not try to get the doc lines', function () {
-          return this.DocumentManager.getDoc.called.should.equal(false)
+        it('should not try to get the doc lines', function () {
+          this.DocumentManager.getDoc.called.should.equal(false)
         })
       })
 
-      return describe('with the undoing flag', function () {
+      describe('with the undoing flag', function () {
         beforeEach(function () {
           // Copy ops so we don't interfere with other tests
           this.ops = [
@@ -641,7 +607,7 @@ describe('DocumentManager', function () {
           this.DiffCodec.diffAsShareJsOp = sinon
             .stub()
             .callsArgWith(2, null, this.ops)
-          return this.DocumentManager.setDoc(
+          this.DocumentManager.setDoc(
             this.project_id,
             this.doc_id,
             this.afterLines,
@@ -652,8 +618,8 @@ describe('DocumentManager', function () {
           )
         })
 
-        return it('should set the undo flag on each op', function () {
-          return Array.from(this.ops).map(op => op.u.should.equal(true))
+        it('should set the undo flag on each op', function () {
+          this.ops.map(op => op.u.should.equal(true))
         })
       })
     })
@@ -678,12 +644,12 @@ describe('DocumentManager', function () {
       this.RangesManager.acceptChanges = sinon
         .stub()
         .yields(null, this.updated_ranges)
-      return (this.RedisManager.updateDocument = sinon.stub().yields())
+      this.RedisManager.updateDocument = sinon.stub().yields()
     })
 
     describe('successfully with a single change', function () {
       beforeEach(function () {
-        return this.DocumentManager.acceptChanges(
+        this.DocumentManager.acceptChanges(
           this.project_id,
           this.doc_id,
           [this.change_id],
@@ -692,19 +658,19 @@ describe('DocumentManager', function () {
       })
 
       it("should get the document's current ranges", function () {
-        return this.DocumentManager.getDoc
+        this.DocumentManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should apply the accept change to the ranges', function () {
-        return this.RangesManager.acceptChanges
+        this.RangesManager.acceptChanges
           .calledWith([this.change_id], this.ranges)
           .should.equal(true)
       })
 
       it('should save the updated ranges', function () {
-        return this.RedisManager.updateDocument
+        this.RedisManager.updateDocument
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -717,14 +683,14 @@ describe('DocumentManager', function () {
           .should.equal(true)
       })
 
-      return it('should call the callback', function () {
-        return this.callback.called.should.equal(true)
+      it('should call the callback', function () {
+        this.callback.called.should.equal(true)
       })
     })
 
     describe('successfully with multiple changes', function () {
       beforeEach(function () {
-        return this.DocumentManager.acceptChanges(
+        this.DocumentManager.acceptChanges(
           this.project_id,
           this.doc_id,
           this.change_ids,
@@ -732,19 +698,19 @@ describe('DocumentManager', function () {
         )
       })
 
-      return it('should apply the accept change to the ranges', function () {
-        return this.RangesManager.acceptChanges
+      it('should apply the accept change to the ranges', function () {
+        this.RangesManager.acceptChanges
           .calledWith(this.change_ids, this.ranges)
           .should.equal(true)
       })
     })
 
-    return describe('when the doc is not found', function () {
+    describe('when the doc is not found', function () {
       beforeEach(function () {
         this.DocumentManager.getDoc = sinon
           .stub()
           .yields(null, null, null, null)
-        return this.DocumentManager.acceptChanges(
+        this.DocumentManager.acceptChanges(
           this.project_id,
           this.doc_id,
           [this.change_id],
@@ -753,11 +719,11 @@ describe('DocumentManager', function () {
       })
 
       it('should not save anything', function () {
-        return this.RedisManager.updateDocument.called.should.equal(false)
+        this.RedisManager.updateDocument.called.should.equal(false)
       })
 
-      return it('should call the callback with a not found error', function () {
-        return this.callback
+      it('should call the callback with a not found error', function () {
+        this.callback
           .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
           .should.equal(true)
       })
@@ -777,12 +743,12 @@ describe('DocumentManager', function () {
       this.RangesManager.deleteComment = sinon
         .stub()
         .yields(null, this.updated_ranges)
-      return (this.RedisManager.updateDocument = sinon.stub().yields())
+      this.RedisManager.updateDocument = sinon.stub().yields()
     })
 
     describe('successfully', function () {
       beforeEach(function () {
-        return this.DocumentManager.deleteComment(
+        this.DocumentManager.deleteComment(
           this.project_id,
           this.doc_id,
           this.comment_id,
@@ -791,19 +757,19 @@ describe('DocumentManager', function () {
       })
 
       it("should get the document's current ranges", function () {
-        return this.DocumentManager.getDoc
+        this.DocumentManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should delete the comment from the ranges', function () {
-        return this.RangesManager.deleteComment
+        this.RangesManager.deleteComment
           .calledWith(this.comment_id, this.ranges)
           .should.equal(true)
       })
 
       it('should save the updated ranges', function () {
-        return this.RedisManager.updateDocument
+        this.RedisManager.updateDocument
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -816,17 +782,17 @@ describe('DocumentManager', function () {
           .should.equal(true)
       })
 
-      return it('should call the callback', function () {
-        return this.callback.called.should.equal(true)
+      it('should call the callback', function () {
+        this.callback.called.should.equal(true)
       })
     })
 
-    return describe('when the doc is not found', function () {
+    describe('when the doc is not found', function () {
       beforeEach(function () {
         this.DocumentManager.getDoc = sinon
           .stub()
           .yields(null, null, null, null)
-        return this.DocumentManager.acceptChanges(
+        this.DocumentManager.acceptChanges(
           this.project_id,
           this.doc_id,
           [this.comment_id],
@@ -835,11 +801,11 @@ describe('DocumentManager', function () {
       })
 
       it('should not save anything', function () {
-        return this.RedisManager.updateDocument.called.should.equal(false)
+        this.RedisManager.updateDocument.called.should.equal(false)
       })
 
-      return it('should call the callback with a not found error', function () {
-        return this.callback
+      it('should call the callback with a not found error', function () {
+        this.callback
           .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
           .should.equal(true)
       })
@@ -848,7 +814,7 @@ describe('DocumentManager', function () {
 
   describe('getDocAndFlushIfOld', function () {
     beforeEach(function () {
-      return (this.DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2))
+      this.DocumentManager.flushDocIfLoaded = sinon.stub().callsArg(2)
     })
 
     describe('when the doc is in Redis', function () {
@@ -867,7 +833,7 @@ describe('DocumentManager', function () {
               Date.now() - 1e9,
               true
             )
-          return this.DocumentManager.getDocAndFlushIfOld(
+          this.DocumentManager.getDocAndFlushIfOld(
             this.project_id,
             this.doc_id,
             this.callback
@@ -875,25 +841,25 @@ describe('DocumentManager', function () {
         })
 
         it('should get the doc', function () {
-          return this.DocumentManager.getDoc
+          this.DocumentManager.getDoc
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
         })
 
         it('should flush the doc', function () {
-          return this.DocumentManager.flushDocIfLoaded
+          this.DocumentManager.flushDocIfLoaded
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
         })
 
-        return it('should call the callback with the lines and versions', function () {
-          return this.callback
+        it('should call the callback with the lines and versions', function () {
+          this.callback
             .calledWith(null, this.lines, this.version)
             .should.equal(true)
         })
       })
 
-      return describe("and has only changes that don't need to be flushed", function () {
+      describe("and has only changes that don't need to be flushed", function () {
         beforeEach(function () {
           this.DocumentManager.getDoc = sinon
             .stub()
@@ -907,7 +873,7 @@ describe('DocumentManager', function () {
               Date.now() - 100,
               true
             )
-          return this.DocumentManager.getDocAndFlushIfOld(
+          this.DocumentManager.getDocAndFlushIfOld(
             this.project_id,
             this.doc_id,
             this.callback
@@ -915,26 +881,24 @@ describe('DocumentManager', function () {
         })
 
         it('should get the doc', function () {
-          return this.DocumentManager.getDoc
+          this.DocumentManager.getDoc
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
         })
 
         it('should not flush the doc', function () {
-          return this.DocumentManager.flushDocIfLoaded.called.should.equal(
-            false
-          )
+          this.DocumentManager.flushDocIfLoaded.called.should.equal(false)
         })
 
-        return it('should call the callback with the lines and versions', function () {
-          return this.callback
+        it('should call the callback with the lines and versions', function () {
+          this.callback
             .calledWith(null, this.lines, this.version)
             .should.equal(true)
         })
       })
     })
 
-    return describe('when the doc is not in Redis', function () {
+    describe('when the doc is not in Redis', function () {
       beforeEach(function () {
         this.DocumentManager.getDoc = sinon
           .stub()
@@ -947,7 +911,7 @@ describe('DocumentManager', function () {
             null,
             false
           )
-        return this.DocumentManager.getDocAndFlushIfOld(
+        this.DocumentManager.getDocAndFlushIfOld(
           this.project_id,
           this.doc_id,
           this.callback
@@ -955,17 +919,17 @@ describe('DocumentManager', function () {
       })
 
       it('should get the doc', function () {
-        return this.DocumentManager.getDoc
+        this.DocumentManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('should not flush the doc', function () {
-        return this.DocumentManager.flushDocIfLoaded.called.should.equal(false)
+        this.DocumentManager.flushDocIfLoaded.called.should.equal(false)
       })
 
-      return it('should call the callback with the lines and versions', function () {
-        return this.callback
+      it('should call the callback with the lines and versions', function () {
+        this.callback
           .calledWith(null, this.lines, this.version)
           .should.equal(true)
       })
@@ -975,12 +939,12 @@ describe('DocumentManager', function () {
   describe('renameDoc', function () {
     beforeEach(function () {
       this.update = 'some-update'
-      return (this.RedisManager.renameDoc = sinon.stub().yields())
+      this.RedisManager.renameDoc = sinon.stub().yields()
     })
 
-    return describe('successfully', function () {
+    describe('successfully', function () {
       beforeEach(function () {
-        return this.DocumentManager.renameDoc(
+        this.DocumentManager.renameDoc(
           this.project_id,
           this.doc_id,
           this.user_id,
@@ -991,7 +955,7 @@ describe('DocumentManager', function () {
       })
 
       it('should rename the document', function () {
-        return this.RedisManager.renameDoc
+        this.RedisManager.renameDoc
           .calledWith(
             this.project_id,
             this.doc_id,
@@ -1002,13 +966,13 @@ describe('DocumentManager', function () {
           .should.equal(true)
       })
 
-      return it('should call the callback', function () {
-        return this.callback.called.should.equal(true)
+      it('should call the callback', function () {
+        this.callback.called.should.equal(true)
       })
     })
   })
 
-  return describe('resyncDocContents', function () {
+  describe('resyncDocContents', function () {
     describe('when doc is loaded in redis', function () {
       beforeEach(function () {
         this.RedisManager.getDoc = sinon
@@ -1023,7 +987,7 @@ describe('DocumentManager', function () {
             this.projectHistoryId
           )
         this.ProjectHistoryRedisManager.queueResyncDocContent = sinon.stub()
-        return this.DocumentManager.resyncDocContents(
+        this.DocumentManager.resyncDocContents(
           this.project_id,
           this.doc_id,
           this.callback
@@ -1031,13 +995,13 @@ describe('DocumentManager', function () {
       })
 
       it('gets the doc contents from redis', function () {
-        return this.RedisManager.getDoc
+        this.RedisManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
-      return it('queues a resync doc content update', function () {
-        return this.ProjectHistoryRedisManager.queueResyncDocContent
+      it('queues a resync doc content update', function () {
+        this.ProjectHistoryRedisManager.queueResyncDocContent
           .calledWith(
             this.project_id,
             this.projectHistoryId,
@@ -1051,13 +1015,12 @@ describe('DocumentManager', function () {
       })
     })
 
-    return describe('when doc is not loaded in redis', function () {
+    describe('when doc is not loaded in redis', function () {
       beforeEach(function () {
         this.RedisManager.getDoc = sinon.stub().callsArgWith(2, null)
         this.PersistenceManager.getDoc = sinon
           .stub()
-          .callsArgWith(
-            2,
+          .yields(
             null,
             this.lines,
             this.version,
@@ -1066,7 +1029,7 @@ describe('DocumentManager', function () {
             this.projectHistoryId
           )
         this.ProjectHistoryRedisManager.queueResyncDocContent = sinon.stub()
-        return this.DocumentManager.resyncDocContents(
+        this.DocumentManager.resyncDocContents(
           this.project_id,
           this.doc_id,
           this.callback
@@ -1074,19 +1037,19 @@ describe('DocumentManager', function () {
       })
 
       it('tries to get the doc contents from redis', function () {
-        return this.RedisManager.getDoc
+        this.RedisManager.getDoc
           .calledWith(this.project_id, this.doc_id)
           .should.equal(true)
       })
 
       it('gets the doc contents from web', function () {
-        return this.PersistenceManager.getDoc
-          .calledWith(this.project_id, this.doc_id)
+        this.PersistenceManager.getDoc
+          .calledWith(this.project_id, this.doc_id, { peek: true })
           .should.equal(true)
       })
 
-      return it('queues a resync doc content update', function () {
-        return this.ProjectHistoryRedisManager.queueResyncDocContent
+      it('queues a resync doc content update', function () {
+        this.ProjectHistoryRedisManager.queueResyncDocContent
           .calledWith(
             this.project_id,
             this.projectHistoryId,

+ 103 - 84
services/document-updater/test/unit/js/PersistenceManager/PersistenceManagerTests.js

@@ -1,15 +1,3 @@
-/* eslint-disable
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS206: Consider reworking classes to avoid initClass
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const sinon = require('sinon')
 const modulePath = '../../../../app/js/PersistenceManager.js'
 const SandboxedModule = require('sandboxed-module')
@@ -17,25 +5,20 @@ const Errors = require('../../../../app/js/Errors')
 
 describe('PersistenceManager', function () {
   beforeEach(function () {
-    let Timer
     this.request = sinon.stub()
     this.request.defaults = () => this.request
+    this.Metrics = {
+      Timer: class Timer {},
+      inc: sinon.stub(),
+    }
+    this.Metrics.Timer.prototype.done = sinon.stub()
+    this.Settings = {}
+
     this.PersistenceManager = SandboxedModule.require(modulePath, {
       requires: {
         requestretry: this.request,
-        '@overleaf/settings': (this.Settings = {}),
-        './Metrics': (this.Metrics = {
-          Timer: (Timer = (function () {
-            Timer = class Timer {
-              static initClass() {
-                this.prototype.done = sinon.stub()
-              }
-            }
-            Timer.initClass()
-            return Timer
-          })()),
-          inc: sinon.stub(),
-        }),
+        '@overleaf/settings': this.Settings,
+        './Metrics': this.Metrics,
         './Errors': Errors,
       },
     })
@@ -49,24 +32,24 @@ describe('PersistenceManager', function () {
     this.pathname = '/a/b/c.tex'
     this.lastUpdatedAt = Date.now()
     this.lastUpdatedBy = 'last-author-id'
-    return (this.Settings.apis = {
+    this.Settings.apis = {
       web: {
         url: (this.url = 'www.example.com'),
         user: (this.user = 'sharelatex'),
         pass: (this.pass = 'password'),
       },
-    })
+    }
   })
 
   describe('getDoc', function () {
     beforeEach(function () {
-      return (this.webResponse = {
+      this.webResponse = {
         lines: this.lines,
         version: this.version,
         ranges: this.ranges,
         pathname: this.pathname,
         projectHistoryId: this.projectHistoryId,
-      })
+      }
     })
 
     describe('with a successful response from the web api', function () {
@@ -77,7 +60,7 @@ describe('PersistenceManager', function () {
           { statusCode: 200 },
           JSON.stringify(this.webResponse)
         )
-        return this.PersistenceManager.getDoc(
+        this.PersistenceManager.getDoc(
           this.project_id,
           this.doc_id,
           this.callback
@@ -85,7 +68,7 @@ describe('PersistenceManager', function () {
       })
 
       it('should call the web api', function () {
-        return this.request
+        this.request
           .calledWith({
             url: `${this.url}/project/${this.project_id}/doc/${this.doc_id}`,
             method: 'GET',
@@ -104,7 +87,7 @@ describe('PersistenceManager', function () {
       })
 
       it('should call the callback with the doc lines, version and ranges', function () {
-        return this.callback
+        this.callback
           .calledWith(
             null,
             this.lines,
@@ -117,22 +100,58 @@ describe('PersistenceManager', function () {
       })
 
       it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
 
-      return it('should increment the metric', function () {
-        return this.Metrics.inc
+      it('should increment the metric', function () {
+        this.Metrics.inc
           .calledWith('getDoc', 1, { status: 200 })
           .should.equal(true)
       })
     })
 
+    describe('with the peek option', function () {
+      beforeEach(function () {
+        this.request.yields(
+          null,
+          { statusCode: 200 },
+          JSON.stringify(this.webResponse)
+        )
+        this.PersistenceManager.getDoc(
+          this.project_id,
+          this.doc_id,
+          { peek: true },
+          this.callback
+        )
+      })
+
+      it('should call the web api with a peek param', function () {
+        this.request
+          .calledWith({
+            url: `${this.url}/project/${this.project_id}/doc/${this.doc_id}`,
+            qs: { peek: 'true' },
+            method: 'GET',
+            headers: {
+              accept: 'application/json',
+            },
+            auth: {
+              user: this.user,
+              pass: this.pass,
+              sendImmediately: true,
+            },
+            jar: false,
+            timeout: 5000,
+          })
+          .should.equal(true)
+      })
+    })
+
     describe('when request returns an error', function () {
       beforeEach(function () {
         this.error = new Error('oops')
         this.error.code = 'EOOPS'
         this.request.callsArgWith(1, this.error, null, null)
-        return this.PersistenceManager.getDoc(
+        this.PersistenceManager.getDoc(
           this.project_id,
           this.doc_id,
           this.callback
@@ -140,7 +159,7 @@ describe('PersistenceManager', function () {
       })
 
       it('should return a generic connection error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Error)
@@ -150,11 +169,11 @@ describe('PersistenceManager', function () {
       })
 
       it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
 
-      return it('should increment the metric', function () {
-        return this.Metrics.inc
+      it('should increment the metric', function () {
+        this.Metrics.inc
           .calledWith('getDoc', 1, { status: 'EOOPS' })
           .should.equal(true)
       })
@@ -163,7 +182,7 @@ describe('PersistenceManager', function () {
     describe('when the request returns 404', function () {
       beforeEach(function () {
         this.request.callsArgWith(1, null, { statusCode: 404 }, '')
-        return this.PersistenceManager.getDoc(
+        this.PersistenceManager.getDoc(
           this.project_id,
           this.doc_id,
           this.callback
@@ -171,17 +190,17 @@ describe('PersistenceManager', function () {
       })
 
       it('should return a NotFoundError', function () {
-        return this.callback
+        this.callback
           .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
           .should.equal(true)
       })
 
       it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
 
-      return it('should increment the metric', function () {
-        return this.Metrics.inc
+      it('should increment the metric', function () {
+        this.Metrics.inc
           .calledWith('getDoc', 1, { status: 404 })
           .should.equal(true)
       })
@@ -190,7 +209,7 @@ describe('PersistenceManager', function () {
     describe('when the request returns an error status code', function () {
       beforeEach(function () {
         this.request.callsArgWith(1, null, { statusCode: 500 }, '')
-        return this.PersistenceManager.getDoc(
+        this.PersistenceManager.getDoc(
           this.project_id,
           this.doc_id,
           this.callback
@@ -198,17 +217,17 @@ describe('PersistenceManager', function () {
       })
 
       it('should return an error', function () {
-        return this.callback
+        this.callback
           .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
 
       it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
 
-      return it('should increment the metric', function () {
-        return this.Metrics.inc
+      it('should increment the metric', function () {
+        this.Metrics.inc
           .calledWith('getDoc', 1, { status: 500 })
           .should.equal(true)
       })
@@ -223,15 +242,15 @@ describe('PersistenceManager', function () {
           { statusCode: 200 },
           JSON.stringify(this.webResponse)
         )
-        return this.PersistenceManager.getDoc(
+        this.PersistenceManager.getDoc(
           this.project_id,
           this.doc_id,
           this.callback
         )
       })
 
-      return it('should return and error', function () {
-        return this.callback
+      it('should return and error', function () {
+        this.callback
           .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
@@ -246,21 +265,21 @@ describe('PersistenceManager', function () {
           { statusCode: 200 },
           JSON.stringify(this.webResponse)
         )
-        return this.PersistenceManager.getDoc(
+        this.PersistenceManager.getDoc(
           this.project_id,
           this.doc_id,
           this.callback
         )
       })
 
-      return it('should return and error', function () {
-        return this.callback
+      it('should return and error', function () {
+        this.callback
           .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
 
-    return describe('when request returns an doc without a pathname', function () {
+    describe('when request returns an doc without a pathname', function () {
       beforeEach(function () {
         delete this.webResponse.pathname
         this.request.callsArgWith(
@@ -269,26 +288,26 @@ describe('PersistenceManager', function () {
           { statusCode: 200 },
           JSON.stringify(this.webResponse)
         )
-        return this.PersistenceManager.getDoc(
+        this.PersistenceManager.getDoc(
           this.project_id,
           this.doc_id,
           this.callback
         )
       })
 
-      return it('should return and error', function () {
-        return this.callback
+      it('should return and error', function () {
+        this.callback
           .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
     })
   })
 
-  return describe('setDoc', function () {
+  describe('setDoc', function () {
     describe('with a successful response from the web api', function () {
       beforeEach(function () {
         this.request.callsArgWith(1, null, { statusCode: 200 })
-        return this.PersistenceManager.setDoc(
+        this.PersistenceManager.setDoc(
           this.project_id,
           this.doc_id,
           this.lines,
@@ -301,7 +320,7 @@ describe('PersistenceManager', function () {
       })
 
       it('should call the web api', function () {
-        return this.request
+        this.request
           .calledWith({
             url: `${this.url}/project/${this.project_id}/doc/${this.doc_id}`,
             json: {
@@ -324,15 +343,15 @@ describe('PersistenceManager', function () {
       })
 
       it('should call the callback without error', function () {
-        return this.callback.calledWith(null).should.equal(true)
+        this.callback.calledWith(null).should.equal(true)
       })
 
       it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
 
-      return it('should increment the metric', function () {
-        return this.Metrics.inc
+      it('should increment the metric', function () {
+        this.Metrics.inc
           .calledWith('setDoc', 1, { status: 200 })
           .should.equal(true)
       })
@@ -343,7 +362,7 @@ describe('PersistenceManager', function () {
         this.error = new Error('oops')
         this.error.code = 'EOOPS'
         this.request.callsArgWith(1, this.error, null, null)
-        return this.PersistenceManager.setDoc(
+        this.PersistenceManager.setDoc(
           this.project_id,
           this.doc_id,
           this.lines,
@@ -356,7 +375,7 @@ describe('PersistenceManager', function () {
       })
 
       it('should return a generic connection error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Error)
@@ -366,11 +385,11 @@ describe('PersistenceManager', function () {
       })
 
       it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
 
-      return it('should increment the metric', function () {
-        return this.Metrics.inc
+      it('should increment the metric', function () {
+        this.Metrics.inc
           .calledWith('setDoc', 1, { status: 'EOOPS' })
           .should.equal(true)
       })
@@ -379,7 +398,7 @@ describe('PersistenceManager', function () {
     describe('when the request returns 404', function () {
       beforeEach(function () {
         this.request.callsArgWith(1, null, { statusCode: 404 }, '')
-        return this.PersistenceManager.setDoc(
+        this.PersistenceManager.setDoc(
           this.project_id,
           this.doc_id,
           this.lines,
@@ -392,26 +411,26 @@ describe('PersistenceManager', function () {
       })
 
       it('should return a NotFoundError', function () {
-        return this.callback
+        this.callback
           .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
           .should.equal(true)
       })
 
       it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
 
-      return it('should increment the metric', function () {
-        return this.Metrics.inc
+      it('should increment the metric', function () {
+        this.Metrics.inc
           .calledWith('setDoc', 1, { status: 404 })
           .should.equal(true)
       })
     })
 
-    return describe('when the request returns an error status code', function () {
+    describe('when the request returns an error status code', function () {
       beforeEach(function () {
         this.request.callsArgWith(1, null, { statusCode: 500 }, '')
-        return this.PersistenceManager.setDoc(
+        this.PersistenceManager.setDoc(
           this.project_id,
           this.doc_id,
           this.lines,
@@ -424,17 +443,17 @@ describe('PersistenceManager', function () {
       })
 
       it('should return an error', function () {
-        return this.callback
+        this.callback
           .calledWith(sinon.match.instanceOf(Error))
           .should.equal(true)
       })
 
       it('should time the execution', function () {
-        return this.Metrics.Timer.prototype.done.called.should.equal(true)
+        this.Metrics.Timer.prototype.done.called.should.equal(true)
       })
 
-      return it('should increment the metric', function () {
-        return this.Metrics.inc
+      it('should increment the metric', function () {
+        this.Metrics.inc
           .calledWith('setDoc', 1, { status: 500 })
           .should.equal(true)
       })

+ 239 - 265
services/web/app/src/Features/Docstore/DocstoreManager.js

@@ -1,303 +1,277 @@
-/* eslint-disable
-    camelcase,
-    node/handle-callback-err,
-    max-len,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
+const { promisify } = require('util')
+const { promisifyMultiResult } = require('../../util/promises')
 const request = require('request').defaults({ jar: false })
 const OError = require('@overleaf/o-error')
 const logger = require('logger-sharelatex')
 const settings = require('@overleaf/settings')
 const Errors = require('../Errors/Errors')
-const { promisifyAll } = require('../../util/promises')
 
 const TIMEOUT = 30 * 1000 // request timeout
 
-const DocstoreManager = {
-  deleteDoc(project_id, doc_id, name, deletedAt, callback) {
-    if (callback == null) {
-      callback = function (error) {}
+function deleteDoc(projectId, docId, name, deletedAt, callback) {
+  const url = `${settings.apis.docstore.url}/project/${projectId}/doc/${docId}`
+  const docMetaData = { deleted: true, deletedAt, name }
+  const options = { url, json: docMetaData, timeout: TIMEOUT }
+  request.patch(options, (error, res) => {
+    if (error) {
+      return callback(error)
     }
-    const url = `${settings.apis.docstore.url}/project/${project_id}/doc/${doc_id}`
-    const docMetaData = { deleted: true, deletedAt, name }
-    const options = { url, json: docMetaData, timeout: TIMEOUT }
-    request.patch(options, function (error, res) {
-      if (error != null) {
+    if (res.statusCode >= 200 && res.statusCode < 300) {
+      callback(null)
+    } else if (res.statusCode === 404) {
+      error = new Errors.NotFoundError({
+        message: 'tried to delete doc not in docstore',
+        info: {
+          projectId,
+          docId,
+        },
+      })
+      callback(error) // maybe suppress the error when delete doc which is not present?
+    } else {
+      error = new OError(
+        `docstore api responded with non-success code: ${res.statusCode}`,
+        {
+          projectId,
+          docId,
+        }
+      )
+      callback(error)
+    }
+  })
+}
+
+function getAllDocs(projectId, callback) {
+  const url = `${settings.apis.docstore.url}/project/${projectId}/doc`
+  request.get(
+    {
+      url,
+      timeout: TIMEOUT,
+      json: true,
+    },
+    (error, res, docs) => {
+      if (error) {
         return callback(error)
       }
       if (res.statusCode >= 200 && res.statusCode < 300) {
-        return callback(null)
-      } else if (res.statusCode === 404) {
-        error = new Errors.NotFoundError({
-          message: 'tried to delete doc not in docstore',
-          info: {
-            project_id,
-            doc_id,
-          },
-        })
-        return callback(error) // maybe suppress the error when delete doc which is not present?
+        callback(null, docs)
       } else {
         error = new OError(
           `docstore api responded with non-success code: ${res.statusCode}`,
-          {
-            project_id,
-            doc_id,
-          }
+          { projectId }
         )
-        return callback(error)
+        callback(error)
       }
-    })
-  },
-
-  getAllDocs(project_id, callback) {
-    if (callback == null) {
-      callback = function (error) {}
     }
-    const url = `${settings.apis.docstore.url}/project/${project_id}/doc`
-    return request.get(
-      {
-        url,
-        timeout: TIMEOUT,
-        json: true,
-      },
-      function (error, res, docs) {
-        if (error != null) {
-          return callback(error)
-        }
-        if (res.statusCode >= 200 && res.statusCode < 300) {
-          return callback(null, docs)
-        } else {
-          error = new OError(
-            `docstore api responded with non-success code: ${res.statusCode}`,
-            { project_id }
-          )
-          return callback(error)
-        }
-      }
-    )
-  },
-
-  getAllDeletedDocs(project_id, callback) {
-    const url = `${settings.apis.docstore.url}/project/${project_id}/doc-deleted`
-    request.get(
-      { url, timeout: TIMEOUT, json: true },
-      function (error, res, docs) {
-        if (error) {
-          callback(
-            OError.tag(error, 'could not get deleted docs from docstore')
-          )
-        } else if (res.statusCode === 200) {
-          callback(null, docs)
-        } else {
-          callback(
-            new OError(
-              `docstore api responded with non-success code: ${res.statusCode}`,
-              { project_id }
-            )
-          )
-        }
-      }
-    )
-  },
+  )
+}
 
-  getAllRanges(project_id, callback) {
-    if (callback == null) {
-      callback = function (error) {}
+function getAllDeletedDocs(projectId, callback) {
+  const url = `${settings.apis.docstore.url}/project/${projectId}/doc-deleted`
+  request.get({ url, timeout: TIMEOUT, json: true }, (error, res, docs) => {
+    if (error) {
+      callback(OError.tag(error, 'could not get deleted docs from docstore'))
+    } else if (res.statusCode === 200) {
+      callback(null, docs)
+    } else {
+      callback(
+        new OError(
+          `docstore api responded with non-success code: ${res.statusCode}`,
+          { projectId }
+        )
+      )
     }
-    const url = `${settings.apis.docstore.url}/project/${project_id}/ranges`
-    return request.get(
-      {
-        url,
-        timeout: TIMEOUT,
-        json: true,
-      },
-      function (error, res, docs) {
-        if (error != null) {
-          return callback(error)
-        }
-        if (res.statusCode >= 200 && res.statusCode < 300) {
-          return callback(null, docs)
-        } else {
-          error = new OError(
-            `docstore api responded with non-success code: ${res.statusCode}`,
-            { project_id }
-          )
-          return callback(error)
-        }
-      }
-    )
-  },
+  })
+}
 
-  getDoc(project_id, doc_id, options, callback) {
-    if (options == null) {
-      options = {}
-    }
-    if (callback == null) {
-      callback = function (error, lines, rev, version) {}
-    }
-    if (typeof options === 'function') {
-      callback = options
-      options = {}
-    }
-    let url = `${settings.apis.docstore.url}/project/${project_id}/doc/${doc_id}`
-    if (options.include_deleted) {
-      url += '?include_deleted=true'
-    }
-    return request.get(
-      {
-        url,
-        timeout: TIMEOUT,
-        json: true,
-      },
-      function (error, res, doc) {
-        if (error != null) {
-          return callback(error)
-        }
-        if (res.statusCode >= 200 && res.statusCode < 300) {
-          logger.log(
-            { doc_id, project_id, version: doc.version, rev: doc.rev },
-            'got doc from docstore api'
-          )
-          return callback(null, doc.lines, doc.rev, doc.version, doc.ranges)
-        } else if (res.statusCode === 404) {
-          error = new Errors.NotFoundError({
-            message: 'doc not found in docstore',
-            info: {
-              project_id,
-              doc_id,
-            },
-          })
-          return callback(error)
-        } else {
-          error = new OError(
-            `docstore api responded with non-success code: ${res.statusCode}`,
-            {
-              project_id,
-              doc_id,
-            }
-          )
-          return callback(error)
-        }
+function getAllRanges(projectId, callback) {
+  const url = `${settings.apis.docstore.url}/project/${projectId}/ranges`
+  request.get(
+    {
+      url,
+      timeout: TIMEOUT,
+      json: true,
+    },
+    (error, res, docs) => {
+      if (error) {
+        return callback(error)
       }
-    )
-  },
-
-  isDocDeleted(project_id, doc_id, callback) {
-    const url = `${settings.apis.docstore.url}/project/${project_id}/doc/${doc_id}/deleted`
-    request.get(
-      { url, timeout: TIMEOUT, json: true },
-      function (err, res, body) {
-        if (err) {
-          callback(err)
-        } else if (res.statusCode === 200) {
-          callback(null, body.deleted)
-        } else if (res.statusCode === 404) {
-          callback(
-            new Errors.NotFoundError({
-              message: 'doc does not exist in project',
-              info: { project_id, doc_id },
-            })
-          )
-        } else {
-          callback(
-            new OError(
-              `docstore api responded with non-success code: ${res.statusCode}`,
-              { project_id, doc_id }
-            )
-          )
-        }
+      if (res.statusCode >= 200 && res.statusCode < 300) {
+        callback(null, docs)
+      } else {
+        error = new OError(
+          `docstore api responded with non-success code: ${res.statusCode}`,
+          { projectId }
+        )
+        callback(error)
       }
-    )
-  },
+    }
+  )
+}
 
-  updateDoc(project_id, doc_id, lines, version, ranges, callback) {
-    if (callback == null) {
-      callback = function (error, modified, rev) {}
+function getDoc(projectId, docId, options, callback) {
+  if (options == null) {
+    options = {}
+  }
+  if (typeof options === 'function') {
+    callback = options
+    options = {}
+  }
+  const requestParams = { timeout: TIMEOUT, json: true }
+  if (options.peek) {
+    requestParams.url = `${settings.apis.docstore.url}/project/${projectId}/doc/${docId}/peek`
+  } else {
+    requestParams.url = `${settings.apis.docstore.url}/project/${projectId}/doc/${docId}`
+  }
+  if (options.include_deleted) {
+    requestParams.qs = { include_deleted: 'true' }
+  }
+  request.get(requestParams, (error, res, doc) => {
+    if (error) {
+      return callback(error)
     }
-    const url = `${settings.apis.docstore.url}/project/${project_id}/doc/${doc_id}`
-    return request.post(
-      {
-        url,
-        timeout: TIMEOUT,
-        json: {
-          lines,
-          version,
-          ranges,
+    if (res.statusCode >= 200 && res.statusCode < 300) {
+      logger.log(
+        { docId, projectId, version: doc.version, rev: doc.rev },
+        'got doc from docstore api'
+      )
+      callback(null, doc.lines, doc.rev, doc.version, doc.ranges)
+    } else if (res.statusCode === 404) {
+      error = new Errors.NotFoundError({
+        message: 'doc not found in docstore',
+        info: {
+          projectId,
+          docId,
         },
-      },
-      function (error, res, result) {
-        if (error != null) {
-          return callback(error)
+      })
+      callback(error)
+    } else {
+      error = new OError(
+        `docstore api responded with non-success code: ${res.statusCode}`,
+        {
+          projectId,
+          docId,
         }
-        if (res.statusCode >= 200 && res.statusCode < 300) {
-          logger.log(
-            { project_id, doc_id },
-            'update doc in docstore url finished'
-          )
-          return callback(null, result.modified, result.rev)
-        } else {
-          error = new OError(
-            `docstore api responded with non-success code: ${res.statusCode}`,
-            { project_id, doc_id }
-          )
-          return callback(error)
-        }
-      }
-    )
-  },
-
-  archiveProject(project_id, callback) {
-    DocstoreManager._operateOnProject(project_id, 'archive', callback)
-  },
-
-  unarchiveProject(project_id, callback) {
-    DocstoreManager._operateOnProject(project_id, 'unarchive', callback)
-  },
-
-  destroyProject(project_id, callback) {
-    DocstoreManager._operateOnProject(project_id, 'destroy', callback)
-  },
+      )
+      callback(error)
+    }
+  })
+}
 
-  _operateOnProject(project_id, method, callback) {
-    const url = `${settings.apis.docstore.url}/project/${project_id}/${method}`
-    logger.log({ project_id }, `calling ${method} for project in docstore`)
-    // use default timeout for archiving/unarchiving/destroying
-    request.post(url, function (err, res, docs) {
-      if (err != null) {
-        OError.tag(err, `error calling ${method} project in docstore`, {
-          project_id,
+function isDocDeleted(projectId, docId, callback) {
+  const url = `${settings.apis.docstore.url}/project/${projectId}/doc/${docId}/deleted`
+  request.get({ url, timeout: TIMEOUT, json: true }, (err, res, body) => {
+    if (err) {
+      callback(err)
+    } else if (res.statusCode === 200) {
+      callback(null, body.deleted)
+    } else if (res.statusCode === 404) {
+      callback(
+        new Errors.NotFoundError({
+          message: 'doc does not exist in project',
+          info: { projectId, docId },
         })
-        return callback(err)
-      }
+      )
+    } else {
+      callback(
+        new OError(
+          `docstore api responded with non-success code: ${res.statusCode}`,
+          { projectId, docId }
+        )
+      )
+    }
+  })
+}
 
+function updateDoc(projectId, docId, lines, version, ranges, callback) {
+  const url = `${settings.apis.docstore.url}/project/${projectId}/doc/${docId}`
+  request.post(
+    {
+      url,
+      timeout: TIMEOUT,
+      json: {
+        lines,
+        version,
+        ranges,
+      },
+    },
+    (error, res, result) => {
+      if (error) {
+        return callback(error)
+      }
       if (res.statusCode >= 200 && res.statusCode < 300) {
-        callback()
+        logger.log({ projectId, docId }, 'update doc in docstore url finished')
+        callback(null, result.modified, result.rev)
       } else {
-        const error = new Error(
-          `docstore api responded with non-success code: ${res.statusCode}`
-        )
-        logger.warn(
-          { err: error, project_id },
-          `error calling ${method} project in docstore`
+        error = new OError(
+          `docstore api responded with non-success code: ${res.statusCode}`,
+          { projectId, docId }
         )
         callback(error)
       }
-    })
-  },
+    }
+  )
+}
+
+function archiveProject(projectId, callback) {
+  _operateOnProject(projectId, 'archive', callback)
 }
 
-module.exports = DocstoreManager
-module.exports.promises = promisifyAll(DocstoreManager, {
-  multiResult: {
-    getDoc: ['lines', 'rev', 'version', 'ranges'],
-    updateDoc: ['modified', 'rev'],
+function unarchiveProject(projectId, callback) {
+  _operateOnProject(projectId, 'unarchive', callback)
+}
+
+function destroyProject(projectId, callback) {
+  _operateOnProject(projectId, 'destroy', callback)
+}
+
+function _operateOnProject(projectId, method, callback) {
+  const url = `${settings.apis.docstore.url}/project/${projectId}/${method}`
+  logger.log({ projectId }, `calling ${method} for project in docstore`)
+  // use default timeout for archiving/unarchiving/destroying
+  request.post(url, (err, res, docs) => {
+    if (err) {
+      OError.tag(err, `error calling ${method} project in docstore`, {
+        projectId,
+      })
+      return callback(err)
+    }
+
+    if (res.statusCode >= 200 && res.statusCode < 300) {
+      callback()
+    } else {
+      const error = new Error(
+        `docstore api responded with non-success code: ${res.statusCode}`
+      )
+      logger.warn(
+        { err: error, projectId },
+        `error calling ${method} project in docstore`
+      )
+      callback(error)
+    }
+  })
+}
+
+module.exports = {
+  deleteDoc,
+  getAllDocs,
+  getAllDeletedDocs,
+  getAllRanges,
+  getDoc,
+  isDocDeleted,
+  updateDoc,
+  archiveProject,
+  unarchiveProject,
+  destroyProject,
+  promises: {
+    deleteDoc: promisify(deleteDoc),
+    getAllDocs: promisify(getAllDocs),
+    getAllDeletedDocs: promisify(getAllDeletedDocs),
+    getAllRanges: promisify(getAllRanges),
+    getDoc: promisifyMultiResult(getDoc, ['lines', 'rev', 'version', 'ranges']),
+    isDocDeleted: promisify(isDocDeleted),
+    updateDoc: promisifyMultiResult(updateDoc, ['modified', 'rev']),
+    archiveProject: promisify(archiveProject),
+    unarchiveProject: promisify(unarchiveProject),
+    destroyProject: promisify(destroyProject),
   },
-})
+}

+ 101 - 128
services/web/app/src/Features/Documents/DocumentController.js

@@ -1,17 +1,3 @@
-/* eslint-disable
-    camelcase,
-    node/handle-callback-err,
-    max-len,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS103: Rewrite code to no longer use __guard__
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const ProjectGetter = require('../Project/ProjectGetter')
 const OError = require('@overleaf/o-error')
 const ProjectLocator = require('../Project/ProjectLocator')
@@ -20,122 +6,109 @@ const ProjectEntityUpdateHandler = require('../Project/ProjectEntityUpdateHandle
 const logger = require('logger-sharelatex')
 const _ = require('lodash')
 
-module.exports = {
-  getDocument(req, res, next) {
-    if (next == null) {
-      next = function (error) {}
-    }
-    const project_id = req.params.Project_id
-    const { doc_id } = req.params
-    const plain =
-      __guard__(req != null ? req.query : undefined, x => x.plain) === 'true'
-    return ProjectGetter.getProject(
-      project_id,
-      { rootFolder: true, overleaf: true },
-      function (error, project) {
-        if (error != null) {
-          return next(error)
-        }
-        if (project == null) {
-          return res.sendStatus(404)
-        }
-        return ProjectLocator.findElement(
-          { project, element_id: doc_id, type: 'doc' },
-          function (error, doc, path) {
-            if (error != null) {
-              OError.tag(error, 'error finding element for getDocument', {
-                doc_id,
-                project_id,
-              })
-              return next(error)
-            }
-            return ProjectEntityHandler.getDoc(
-              project_id,
-              doc_id,
-              function (error, lines, rev, version, ranges) {
-                if (error != null) {
-                  OError.tag(
-                    error,
-                    'error finding doc contents for getDocument',
-                    {
-                      doc_id,
-                      project_id,
-                    }
-                  )
-                  return next(error)
-                }
-                if (plain) {
-                  res.type('text/plain')
-                  return res.send(lines.join('\n'))
-                } else {
-                  const projectHistoryId = _.get(project, 'overleaf.history.id')
-                  const projectHistoryDisplay = _.get(
-                    project,
-                    'overleaf.history.display'
-                  )
-                  const sendToBothHistorySystems = _.get(
-                    project,
-                    'overleaf.history.allowDowngrade'
-                  )
-                  // if project has been switched but has 'allowDowngrade' set
-                  // then leave projectHistoryType undefined to (temporarily)
-                  // continue sending updates to both SL and full project history
-                  const projectHistoryType =
-                    projectHistoryDisplay && !sendToBothHistorySystems
-                      ? 'project-history'
-                      : undefined // for backwards compatibility, don't send anything if the project is still on track-changes
-                  return res.json({
-                    lines,
-                    version,
-                    ranges,
-                    pathname: path.fileSystem,
-                    projectHistoryId,
-                    projectHistoryType,
-                  })
-                }
-              }
-            )
-          }
-        )
+function getDocument(req, res, next) {
+  const { Project_id: projectId, doc_id: docId } = req.params
+  const plain = req.query.plain === 'true'
+  const peek = req.query.peek === 'true'
+  ProjectGetter.getProject(
+    projectId,
+    { rootFolder: true, overleaf: true },
+    (error, project) => {
+      if (error) {
+        return next(error)
       }
-    )
-  },
-
-  setDocument(req, res, next) {
-    if (next == null) {
-      next = function (error) {}
-    }
-    const project_id = req.params.Project_id
-    const { doc_id } = req.params
-    const { lines, version, ranges, lastUpdatedAt, lastUpdatedBy } = req.body
-    return ProjectEntityUpdateHandler.updateDocLines(
-      project_id,
-      doc_id,
-      lines,
-      version,
-      ranges,
-      lastUpdatedAt,
-      lastUpdatedBy,
-      function (error) {
-        if (error != null) {
-          OError.tag(error, 'error finding element for getDocument', {
-            doc_id,
-            project_id,
-          })
-          return next(error)
-        }
-        logger.log(
-          { doc_id, project_id },
-          'finished receiving set document request from api (docupdater)'
-        )
-        return res.sendStatus(200)
+      if (!project) {
+        return res.sendStatus(404)
       }
-    )
-  },
+      ProjectLocator.findElement(
+        { project, element_id: docId, type: 'doc' },
+        (error, doc, path) => {
+          if (error) {
+            OError.tag(error, 'error finding element for getDocument', {
+              docId,
+              projectId,
+            })
+            return next(error)
+          }
+          ProjectEntityHandler.getDoc(
+            projectId,
+            docId,
+            { peek },
+            (error, lines, rev, version, ranges) => {
+              if (error) {
+                OError.tag(
+                  error,
+                  'error finding doc contents for getDocument',
+                  {
+                    docId,
+                    projectId,
+                  }
+                )
+                return next(error)
+              }
+              if (plain) {
+                res.type('text/plain')
+                res.send(lines.join('\n'))
+              } else {
+                const projectHistoryId = _.get(project, 'overleaf.history.id')
+                const projectHistoryDisplay = _.get(
+                  project,
+                  'overleaf.history.display'
+                )
+                const sendToBothHistorySystems = _.get(
+                  project,
+                  'overleaf.history.allowDowngrade'
+                )
+                // if project has been switched but has 'allowDowngrade' set
+                // then leave projectHistoryType undefined to (temporarily)
+                // continue sending updates to both SL and full project history
+                const projectHistoryType =
+                  projectHistoryDisplay && !sendToBothHistorySystems
+                    ? 'project-history'
+                    : undefined // for backwards compatibility, don't send anything if the project is still on track-changes
+                res.json({
+                  lines,
+                  version,
+                  ranges,
+                  pathname: path.fileSystem,
+                  projectHistoryId,
+                  projectHistoryType,
+                })
+              }
+            }
+          )
+        }
+      )
+    }
+  )
 }
 
-function __guard__(value, transform) {
-  return typeof value !== 'undefined' && value !== null
-    ? transform(value)
-    : undefined
+function setDocument(req, res, next) {
+  const { Project_id: projectId, doc_id: docId } = req.params
+  const { lines, version, ranges, lastUpdatedAt, lastUpdatedBy } = req.body
+  ProjectEntityUpdateHandler.updateDocLines(
+    projectId,
+    docId,
+    lines,
+    version,
+    ranges,
+    lastUpdatedAt,
+    lastUpdatedBy,
+    error => {
+      if (error) {
+        OError.tag(error, 'error finding element for getDocument', {
+          docId,
+          projectId,
+        })
+        return next(error)
+      }
+      logger.log(
+        { docId, projectId },
+        'finished receiving set document request from api (docupdater)'
+      )
+      res.sendStatus(200)
+    }
+  )
 }
+
+module.exports = { getDocument, setDocument }

+ 76 - 98
services/web/test/unit/src/Docstore/DocstoreManagerTests.js

@@ -1,14 +1,3 @@
-/* eslint-disable
-    max-len,
-    no-return-assign,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const sinon = require('sinon')
 const modulePath = '../../../../app/src/Features/Docstore/DocstoreManager'
 const SandboxedModule = require('sandboxed-module')
@@ -37,7 +26,7 @@ describe('DocstoreManager', function () {
 
     this.project_id = 'project-id-123'
     this.doc_id = 'doc-id-123'
-    return (this.callback = sinon.stub())
+    this.callback = sinon.stub()
   })
 
   describe('deleteDoc', function () {
@@ -54,7 +43,7 @@ describe('DocstoreManager', function () {
         this.request.patch = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 204 }, '')
-        return this.DocstoreManager.deleteDoc(
+        this.DocstoreManager.deleteDoc(
           this.project_id,
           this.doc_id,
           'wombat.tex',
@@ -64,7 +53,7 @@ describe('DocstoreManager', function () {
       })
 
       it('should delete the doc in the docstore api', function () {
-        return this.request.patch
+        this.request.patch
           .calledWith({
             url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`,
             json: { deleted: true, deletedAt: new Date(), name: 'wombat.tex' },
@@ -74,7 +63,7 @@ describe('DocstoreManager', function () {
       })
 
       it('should call the callback without an error', function () {
-        return this.callback.calledWith(null).should.equal(true)
+        this.callback.calledWith(null).should.equal(true)
       })
     })
 
@@ -83,7 +72,7 @@ describe('DocstoreManager', function () {
         this.request.patch = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 500 }, '')
-        return this.DocstoreManager.deleteDoc(
+        this.DocstoreManager.deleteDoc(
           this.project_id,
           this.doc_id,
           'main.tex',
@@ -93,7 +82,7 @@ describe('DocstoreManager', function () {
       })
 
       it('should call the callback with an error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Error)
@@ -113,7 +102,7 @@ describe('DocstoreManager', function () {
         this.request.patch = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 404 }, '')
-        return this.DocstoreManager.deleteDoc(
+        this.DocstoreManager.deleteDoc(
           this.project_id,
           this.doc_id,
           'main.tex',
@@ -145,7 +134,7 @@ describe('DocstoreManager', function () {
       this.rev = 5
       this.version = 42
       this.ranges = { mock: 'ranges' }
-      return (this.modified = true)
+      this.modified = true
     })
 
     describe('with a successful response code', function () {
@@ -158,7 +147,7 @@ describe('DocstoreManager', function () {
             { statusCode: 204 },
             { modified: this.modified, rev: this.rev }
           )
-        return this.DocstoreManager.updateDoc(
+        this.DocstoreManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.lines,
@@ -169,7 +158,7 @@ describe('DocstoreManager', function () {
       })
 
       it('should update the doc in the docstore api', function () {
-        return this.request.post
+        this.request.post
           .calledWith({
             url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`,
             timeout: 30 * 1000,
@@ -183,7 +172,7 @@ describe('DocstoreManager', function () {
       })
 
       it('should call the callback with the modified status and revision', function () {
-        return this.callback
+        this.callback
           .calledWith(null, this.modified, this.rev)
           .should.equal(true)
       })
@@ -194,7 +183,7 @@ describe('DocstoreManager', function () {
         this.request.post = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 500 }, '')
-        return this.DocstoreManager.updateDoc(
+        this.DocstoreManager.updateDoc(
           this.project_id,
           this.doc_id,
           this.lines,
@@ -205,7 +194,7 @@ describe('DocstoreManager', function () {
       })
 
       it('should call the callback with an error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Error)
@@ -223,12 +212,12 @@ describe('DocstoreManager', function () {
 
   describe('getDoc', function () {
     beforeEach(function () {
-      return (this.doc = {
+      this.doc = {
         lines: (this.lines = ['mock', 'doc', 'lines']),
         rev: (this.rev = 5),
         version: (this.version = 42),
         ranges: (this.ranges = { mock: 'ranges' }),
-      })
+      }
     })
 
     describe('with a successful response code', function () {
@@ -236,25 +225,19 @@ describe('DocstoreManager', function () {
         this.request.get = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 204 }, this.doc)
-        return this.DocstoreManager.getDoc(
-          this.project_id,
-          this.doc_id,
-          this.callback
-        )
+        this.DocstoreManager.getDoc(this.project_id, this.doc_id, this.callback)
       })
 
       it('should get the doc from the docstore api', function () {
-        return this.request.get
-          .calledWith({
-            url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`,
-            timeout: 30 * 1000,
-            json: true,
-          })
-          .should.equal(true)
+        this.request.get.should.have.been.calledWith({
+          url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`,
+          timeout: 30 * 1000,
+          json: true,
+        })
       })
 
       it('should call the callback with the lines, version and rev', function () {
-        return this.callback
+        this.callback
           .calledWith(null, this.lines, this.rev, this.version, this.ranges)
           .should.equal(true)
       })
@@ -265,15 +248,11 @@ describe('DocstoreManager', function () {
         this.request.get = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 500 }, '')
-        return this.DocstoreManager.getDoc(
-          this.project_id,
-          this.doc_id,
-          this.callback
-        )
+        this.DocstoreManager.getDoc(this.project_id, this.doc_id, this.callback)
       })
 
       it('should call the callback with an error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Error)
@@ -293,7 +272,7 @@ describe('DocstoreManager', function () {
         this.request.get = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 204 }, this.doc)
-        return this.DocstoreManager.getDoc(
+        this.DocstoreManager.getDoc(
           this.project_id,
           this.doc_id,
           { include_deleted: true },
@@ -302,36 +281,53 @@ describe('DocstoreManager', function () {
       })
 
       it('should get the doc from the docstore api (including deleted)', function () {
-        return this.request.get
-          .calledWith({
-            url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}?include_deleted=true`,
-            timeout: 30 * 1000,
-            json: true,
-          })
-          .should.equal(true)
+        this.request.get.should.have.been.calledWith({
+          url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}`,
+          qs: { include_deleted: 'true' },
+          timeout: 30 * 1000,
+          json: true,
+        })
       })
 
       it('should call the callback with the lines, version and rev', function () {
-        return this.callback
+        this.callback
           .calledWith(null, this.lines, this.rev, this.version, this.ranges)
           .should.equal(true)
       })
     })
 
-    describe('with a missing (404) response code', function () {
+    describe('with peek=true', function () {
       beforeEach(function () {
         this.request.get = sinon
           .stub()
-          .callsArgWith(1, null, { statusCode: 404 }, '')
-        return this.DocstoreManager.getDoc(
+          .callsArgWith(1, null, { statusCode: 204 }, this.doc)
+        this.DocstoreManager.getDoc(
           this.project_id,
           this.doc_id,
+          { peek: true },
           this.callback
         )
       })
 
+      it('should call the docstore peek url', function () {
+        this.request.get.should.have.been.calledWith({
+          url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc/${this.doc_id}/peek`,
+          timeout: 30 * 1000,
+          json: true,
+        })
+      })
+    })
+
+    describe('with a missing (404) response code', function () {
+      beforeEach(function () {
+        this.request.get = sinon
+          .stub()
+          .callsArgWith(1, null, { statusCode: 404 }, '')
+        this.DocstoreManager.getDoc(this.project_id, this.doc_id, this.callback)
+      })
+
       it('should call the callback with an error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Errors.NotFoundError)
@@ -353,11 +349,11 @@ describe('DocstoreManager', function () {
             { statusCode: 204 },
             (this.docs = [{ _id: 'mock-doc-id' }])
           )
-        return this.DocstoreManager.getAllDocs(this.project_id, this.callback)
+        this.DocstoreManager.getAllDocs(this.project_id, this.callback)
       })
 
       it('should get all the project docs in the docstore api', function () {
-        return this.request.get
+        this.request.get
           .calledWith({
             url: `${this.settings.apis.docstore.url}/project/${this.project_id}/doc`,
             timeout: 30 * 1000,
@@ -367,7 +363,7 @@ describe('DocstoreManager', function () {
       })
 
       it('should call the callback with the docs', function () {
-        return this.callback.calledWith(null, this.docs).should.equal(true)
+        this.callback.calledWith(null, this.docs).should.equal(true)
       })
     })
 
@@ -376,11 +372,11 @@ describe('DocstoreManager', function () {
         this.request.get = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 500 }, '')
-        return this.DocstoreManager.getAllDocs(this.project_id, this.callback)
+        this.DocstoreManager.getAllDocs(this.project_id, this.callback)
       })
 
       it('should call the callback with an error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Error)
@@ -473,11 +469,11 @@ describe('DocstoreManager', function () {
             { statusCode: 204 },
             (this.docs = [{ _id: 'mock-doc-id', ranges: 'mock-ranges' }])
           )
-        return this.DocstoreManager.getAllRanges(this.project_id, this.callback)
+        this.DocstoreManager.getAllRanges(this.project_id, this.callback)
       })
 
       it('should get all the project doc ranges in the docstore api', function () {
-        return this.request.get
+        this.request.get
           .calledWith({
             url: `${this.settings.apis.docstore.url}/project/${this.project_id}/ranges`,
             timeout: 30 * 1000,
@@ -487,7 +483,7 @@ describe('DocstoreManager', function () {
       })
 
       it('should call the callback with the docs', function () {
-        return this.callback.calledWith(null, this.docs).should.equal(true)
+        this.callback.calledWith(null, this.docs).should.equal(true)
       })
     })
 
@@ -496,11 +492,11 @@ describe('DocstoreManager', function () {
         this.request.get = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 500 }, '')
-        return this.DocstoreManager.getAllRanges(this.project_id, this.callback)
+        this.DocstoreManager.getAllRanges(this.project_id, this.callback)
       })
 
       it('should call the callback with an error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Error)
@@ -522,14 +518,11 @@ describe('DocstoreManager', function () {
         this.request.post = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 204 })
-        return this.DocstoreManager.archiveProject(
-          this.project_id,
-          this.callback
-        )
+        this.DocstoreManager.archiveProject(this.project_id, this.callback)
       })
 
       it('should call the callback', function () {
-        return this.callback.called.should.equal(true)
+        this.callback.called.should.equal(true)
       })
     })
 
@@ -538,14 +531,11 @@ describe('DocstoreManager', function () {
         this.request.post = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 500 })
-        return this.DocstoreManager.archiveProject(
-          this.project_id,
-          this.callback
-        )
+        this.DocstoreManager.archiveProject(this.project_id, this.callback)
       })
 
       it('should call the callback with an error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Error)
@@ -567,14 +557,11 @@ describe('DocstoreManager', function () {
         this.request.post = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 204 })
-        return this.DocstoreManager.unarchiveProject(
-          this.project_id,
-          this.callback
-        )
+        this.DocstoreManager.unarchiveProject(this.project_id, this.callback)
       })
 
       it('should call the callback', function () {
-        return this.callback.called.should.equal(true)
+        this.callback.called.should.equal(true)
       })
     })
 
@@ -583,14 +570,11 @@ describe('DocstoreManager', function () {
         this.request.post = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 500 })
-        return this.DocstoreManager.unarchiveProject(
-          this.project_id,
-          this.callback
-        )
+        this.DocstoreManager.unarchiveProject(this.project_id, this.callback)
       })
 
       it('should call the callback with an error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Error)
@@ -612,14 +596,11 @@ describe('DocstoreManager', function () {
         this.request.post = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 204 })
-        return this.DocstoreManager.destroyProject(
-          this.project_id,
-          this.callback
-        )
+        this.DocstoreManager.destroyProject(this.project_id, this.callback)
       })
 
       it('should call the callback', function () {
-        return this.callback.called.should.equal(true)
+        this.callback.called.should.equal(true)
       })
     })
 
@@ -628,14 +609,11 @@ describe('DocstoreManager', function () {
         this.request.post = sinon
           .stub()
           .callsArgWith(1, null, { statusCode: 500 })
-        return this.DocstoreManager.destroyProject(
-          this.project_id,
-          this.callback
-        )
+        this.DocstoreManager.destroyProject(this.project_id, this.callback)
       })
 
       it('should call the callback with an error', function () {
-        return this.callback
+        this.callback
           .calledWith(
             sinon.match
               .instanceOf(Error)

+ 26 - 85
services/web/test/unit/src/Documents/DocumentControllerTests.js

@@ -1,21 +1,7 @@
-/* eslint-disable
-    max-len,
-    no-return-assign,
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
 const sinon = require('sinon')
-const { expect } = require('chai')
 const modulePath =
   '../../../../app/src/Features/Documents/DocumentController.js'
 const SandboxedModule = require('sandboxed-module')
-const events = require('events')
 const MockRequest = require('../helpers/MockRequest')
 const MockResponse = require('../helpers/MockResponse')
 const Errors = require('../../../../app/src/Features/Errors/Errors')
@@ -41,23 +27,23 @@ describe('DocumentController', function () {
     this.pathname = '/a/b/c/file.tex'
     this.lastUpdatedAt = new Date().getTime()
     this.lastUpdatedBy = 'fake-last-updater-id'
-    return (this.rev = 5)
+    this.rev = 5
   })
 
   describe('getDocument', function () {
     beforeEach(function () {
-      return (this.req.params = {
+      this.req.params = {
         Project_id: this.project_id,
         doc_id: this.doc_id,
-      })
+      }
     })
 
     describe('when the project exists without project history enabled', function () {
       beforeEach(function () {
         this.project = { _id: this.project_id }
-        return (this.ProjectGetter.getProject = sinon
+        this.ProjectGetter.getProject = sinon
           .stub()
-          .callsArgWith(2, null, this.project))
+          .callsArgWith(2, null, this.project)
       })
 
       describe('when the document exists', function () {
@@ -68,29 +54,18 @@ describe('DocumentController', function () {
             .callsArgWith(1, null, this.doc, { fileSystem: this.pathname })
           this.ProjectEntityHandler.getDoc = sinon
             .stub()
-            .callsArgWith(
-              2,
-              null,
-              this.doc_lines,
-              this.rev,
-              this.version,
-              this.ranges
-            )
-          return this.DocumentController.getDocument(
-            this.req,
-            this.res,
-            this.next
-          )
+            .yields(null, this.doc_lines, this.rev, this.version, this.ranges)
+          this.DocumentController.getDocument(this.req, this.res, this.next)
         })
 
         it('should get the project', function () {
-          return this.ProjectGetter.getProject
+          this.ProjectGetter.getProject
             .calledWith(this.project_id, { rootFolder: true, overleaf: true })
             .should.equal(true)
         })
 
         it('should get the pathname of the document', function () {
-          return this.ProjectLocator.findElement
+          this.ProjectLocator.findElement
             .calledWith({
               project: this.project,
               element_id: this.doc_id,
@@ -100,14 +75,14 @@ describe('DocumentController', function () {
         })
 
         it('should get the document content', function () {
-          return this.ProjectEntityHandler.getDoc
+          this.ProjectEntityHandler.getDoc
             .calledWith(this.project_id, this.doc_id)
             .should.equal(true)
         })
 
         it('should return the document data to the client as JSON', function () {
           this.res.type.should.equal('application/json')
-          return this.res.body.should.equal(
+          this.res.body.should.equal(
             JSON.stringify({
               lines: this.doc_lines,
               version: this.version,
@@ -123,15 +98,11 @@ describe('DocumentController', function () {
           this.ProjectLocator.findElement = sinon
             .stub()
             .callsArgWith(1, new Errors.NotFoundError('not found'))
-          return this.DocumentController.getDocument(
-            this.req,
-            this.res,
-            this.next
-          )
+          this.DocumentController.getDocument(this.req, this.res, this.next)
         })
 
         it('should call next with the NotFoundError', function () {
-          return this.next
+          this.next
             .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
             .should.equal(true)
         })
@@ -161,24 +132,13 @@ describe('DocumentController', function () {
           .callsArgWith(1, null, this.doc, { fileSystem: this.pathname })
         this.ProjectEntityHandler.getDoc = sinon
           .stub()
-          .callsArgWith(
-            2,
-            null,
-            this.doc_lines,
-            this.rev,
-            this.version,
-            this.ranges
-          )
-        return this.DocumentController.getDocument(
-          this.req,
-          this.res,
-          this.next
-        )
+          .yields(null, this.doc_lines, this.rev, this.version, this.ranges)
+        this.DocumentController.getDocument(this.req, this.res, this.next)
       })
 
       it('should return the history id and display setting to the client as JSON', function () {
         this.res.type.should.equal('application/json')
-        return this.res.body.should.equal(
+        this.res.body.should.equal(
           JSON.stringify({
             lines: this.doc_lines,
             version: this.version,
@@ -215,14 +175,7 @@ describe('DocumentController', function () {
           .callsArgWith(1, null, this.doc, { fileSystem: this.pathname })
         this.ProjectEntityHandler.getDoc = sinon
           .stub()
-          .callsArgWith(
-            2,
-            null,
-            this.doc_lines,
-            this.rev,
-            this.version,
-            this.ranges
-          )
+          .yields(null, this.doc_lines, this.rev, this.version, this.ranges)
         return this.DocumentController.getDocument(
           this.req,
           this.res,
@@ -248,25 +201,21 @@ describe('DocumentController', function () {
     describe('when the project does not exist', function () {
       beforeEach(function () {
         this.ProjectGetter.getProject = sinon.stub().callsArgWith(2, null, null)
-        return this.DocumentController.getDocument(
-          this.req,
-          this.res,
-          this.next
-        )
+        this.DocumentController.getDocument(this.req, this.res, this.next)
       })
 
       it('returns a 404', function () {
-        return this.res.statusCode.should.equal(404)
+        this.res.statusCode.should.equal(404)
       })
     })
   })
 
   describe('setDocument', function () {
     beforeEach(function () {
-      return (this.req.params = {
+      this.req.params = {
         Project_id: this.project_id,
         doc_id: this.doc_id,
-      })
+      }
     })
 
     describe('when the document exists', function () {
@@ -279,15 +228,11 @@ describe('DocumentController', function () {
           lastUpdatedAt: this.lastUpdatedAt,
           lastUpdatedBy: this.lastUpdatedBy,
         }
-        return this.DocumentController.setDocument(
-          this.req,
-          this.res,
-          this.next
-        )
+        this.DocumentController.setDocument(this.req, this.res, this.next)
       })
 
       it('should update the document in Mongo', function () {
-        return sinon.assert.calledWith(
+        sinon.assert.calledWith(
           this.ProjectEntityUpdateHandler.updateDocLines,
           this.project_id,
           this.doc_id,
@@ -300,7 +245,7 @@ describe('DocumentController', function () {
       })
 
       it('should return a successful response', function () {
-        return this.res.success.should.equal(true)
+        this.res.success.should.equal(true)
       })
     })
 
@@ -310,15 +255,11 @@ describe('DocumentController', function () {
           .stub()
           .yields(new Errors.NotFoundError('document does not exist'))
         this.req.body = { lines: this.doc_lines }
-        return this.DocumentController.setDocument(
-          this.req,
-          this.res,
-          this.next
-        )
+        this.DocumentController.setDocument(this.req, this.res, this.next)
       })
 
       it('should call next with the NotFoundError', function () {
-        return this.next
+        this.next
           .calledWith(sinon.match.instanceOf(Errors.NotFoundError))
           .should.equal(true)
       })