Merge pull request #71 from overleaf/spd-decaf-cleanup-8

Post-decaf cleanup of PersistorManager and KeyBuilderTests

services/filestore/app/js/PersistorManager.js

@@ -1,52 +1,26 @@
-// TODO: This file was created by bulk-decaffeinate.
-// Sanity-check the conversion and remove this comment.
-/*
- * decaffeinate suggestions:
- * DS103: Rewrite code to no longer use __guard__
- * 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
- */
 const settings = require('settings-sharelatex')
 const logger = require('logger-sharelatex')
 
-// assume s3 if none specified
-__guard__(
-  settings != null ? settings.filestore : undefined,
-  x => x.backend || (settings.filestore.backend = 's3')
-)
-
 logger.log(
   {
-    backend: __guard__(
-      settings != null ? settings.filestore : undefined,
-      x1 => x1.backend
-    )
+    backend: settings.filestore.backend
   },
   'Loading backend'
 )
-module.exports = (() => {
-  switch (
-    __guard__(
-      settings != null ? settings.filestore : undefined,
-      x2 => x2.backend
-    )
-  ) {
-    case 'aws-sdk':
-      return require('./AWSSDKPersistorManager')
-    case 's3':
-      return require('./S3PersistorManager')
-    case 'fs':
-      return require('./FSPersistorManager')
-    default:
-      throw new Error(
-        `Unknown filestore backend: ${settings.filestore.backend}`
-      )
-  }
-})()
+if (!settings.filestore.backend) {
+  throw new Error('no backend specified - config incomplete')
+}
 
-function __guard__(value, transform) {
-  return typeof value !== 'undefined' && value !== null
-    ? transform(value)
-    : undefined
+switch (settings.filestore.backend) {
+  case 'aws-sdk':
+    module.exports = require('./AWSSDKPersistorManager')
+    break
+  case 's3':
+    module.exports = require('./S3PersistorManager')
+    break
+  case 'fs':
+    module.exports = require('./FSPersistorManager')
+    break
+  default:
+    throw new Error(`unknown filestore backend: ${settings.filestore.backend}`)
 }

services/filestore/test/unit/js/KeybuilderTests.js

@@ -1,26 +1,13 @@
-/* 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
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
+const SandboxedModule = require('sandboxed-module')
 
-const { assert } = require('chai')
-const sinon = require('sinon')
-const chai = require('chai')
-const should = chai.should()
-const { expect } = chai
 const modulePath = '../../../app/js/KeyBuilder.js'
-const SandboxedModule = require('sandboxed-module')
 
 describe('LocalFileWriter', function() {
+  let KeyBuilder
+  const key = 'wombat/potato'
+
   beforeEach(function() {
-    this.keyBuilder = SandboxedModule.require(modulePath, {
+    KeyBuilder = SandboxedModule.require(modulePath, {
       requires: {
         'logger-sharelatex': {
           log() {},
@@ -28,31 +15,28 @@ describe('LocalFileWriter', function() {
         }
       }
     })
-    return (this.key = '123/456')
   })
 
-  return describe('cachedKey', function() {
-    it('should add the fomat on', function() {
+  describe('cachedKey', function() {
+    it('should add the format to the key', function() {
       const opts = { format: 'png' }
-      const newKey = this.keyBuilder.addCachingToKey(this.key, opts)
-      return newKey.should.equal(`${this.key}-converted-cache/format-png`)
+      const newKey = KeyBuilder.addCachingToKey(key, opts)
+      newKey.should.equal(`${key}-converted-cache/format-png`)
     })
 
-    it('should add the style on', function() {
+    it('should add the style to the key', function() {
       const opts = { style: 'thumbnail' }
-      const newKey = this.keyBuilder.addCachingToKey(this.key, opts)
-      return newKey.should.equal(`${this.key}-converted-cache/style-thumbnail`)
+      const newKey = KeyBuilder.addCachingToKey(key, opts)
+      newKey.should.equal(`${key}-converted-cache/style-thumbnail`)
     })
 
-    return it('should add format on first', function() {
+    it('should add format first, then style', function() {
       const opts = {
         style: 'thumbnail',
         format: 'png'
       }
-      const newKey = this.keyBuilder.addCachingToKey(this.key, opts)
-      return newKey.should.equal(
-        `${this.key}-converted-cache/format-png-style-thumbnail`
-      )
+      const newKey = KeyBuilder.addCachingToKey(key, opts)
+      newKey.should.equal(`${key}-converted-cache/format-png-style-thumbnail`)
     })
   })
 })

services/filestore/test/unit/js/PersistorManagerTests.js

@@ -1,137 +1,74 @@
-/* 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
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const logger = require('logger-sharelatex')
-const { assert } = require('chai')
 const sinon = require('sinon')
 const chai = require('chai')
-const should = chai.should()
 const { expect } = chai
-const modulePath = '../../../app/js/PersistorManager.js'
 const SandboxedModule = require('sandboxed-module')
 
-describe('PersistorManagerTests', function() {
-  beforeEach(function() {
-    return (this.S3PersistorManager = {
-      getFileStream: sinon.stub(),
-      checkIfFileExists: sinon.stub(),
-      deleteFile: sinon.stub(),
-      deleteDirectory: sinon.stub(),
-      sendStream: sinon.stub(),
-      insertFile: sinon.stub()
-    })
-  })
+const modulePath = '../../../app/js/PersistorManager.js'
 
-  describe('test s3 mixin', function() {
-    beforeEach(function() {
-      this.settings = {
-        filestore: {
-          backend: 's3'
-        }
-      }
-      this.requires = {
-        './S3PersistorManager': this.S3PersistorManager,
-        'settings-sharelatex': this.settings,
-        'logger-sharelatex': {
-          log() {},
-          err() {}
-        }
-      }
-      return (this.PersistorManager = SandboxedModule.require(modulePath, {
-        requires: this.requires
-      }))
-    })
+describe('PersistorManager', function() {
+  let PersistorManager,
+    FSPersistorManager,
+    S3PersistorManager,
+    settings,
+    requires
 
-    it('should load getFileStream', function(done) {
-      this.PersistorManager.should.respondTo('getFileStream')
-      this.PersistorManager.getFileStream()
-      this.S3PersistorManager.getFileStream.calledOnce.should.equal(true)
-      return done()
-    })
+  beforeEach(function() {
+    FSPersistorManager = {
+      wrappedMethod: sinon.stub().returns('FSPersistorManager')
+    }
+    S3PersistorManager = {
+      wrappedMethod: sinon.stub().returns('S3PersistorManager')
+    }
 
-    it('should load checkIfFileExists', function(done) {
-      this.PersistorManager.checkIfFileExists()
-      this.S3PersistorManager.checkIfFileExists.calledOnce.should.equal(true)
-      return done()
-    })
+    settings = {
+      filestore: {}
+    }
 
-    it('should load deleteFile', function(done) {
-      this.PersistorManager.deleteFile()
-      this.S3PersistorManager.deleteFile.calledOnce.should.equal(true)
-      return done()
-    })
+    requires = {
+      './S3PersistorManager': S3PersistorManager,
+      './FSPersistorManager': FSPersistorManager,
+      'settings-sharelatex': settings,
+      'logger-sharelatex': {
+        log() {},
+        err() {}
+      }
+    }
+  })
 
-    it('should load deleteDirectory', function(done) {
-      this.PersistorManager.deleteDirectory()
-      this.S3PersistorManager.deleteDirectory.calledOnce.should.equal(true)
-      return done()
-    })
+  it('should implement the S3 wrapped method when S3 is configured', function() {
+    settings.filestore.backend = 's3'
+    PersistorManager = SandboxedModule.require(modulePath, { requires })
 
-    it('should load sendStream', function(done) {
-      this.PersistorManager.sendStream()
-      this.S3PersistorManager.sendStream.calledOnce.should.equal(true)
-      return done()
-    })
+    expect(PersistorManager).to.respondTo('wrappedMethod')
+    expect(PersistorManager.wrappedMethod()).to.equal('S3PersistorManager')
+  })
+
+  it('should implement the FS wrapped method when FS is configured', function() {
+    settings.filestore.backend = 'fs'
+    PersistorManager = SandboxedModule.require(modulePath, { requires })
 
-    return it('should load insertFile', function(done) {
-      this.PersistorManager.insertFile()
-      this.S3PersistorManager.insertFile.calledOnce.should.equal(true)
-      return done()
-    })
+    expect(PersistorManager).to.respondTo('wrappedMethod')
+    expect(PersistorManager.wrappedMethod()).to.equal('FSPersistorManager')
   })
 
-  describe('test unspecified mixins', () =>
-    it('should load s3 when no wrapper specified', function(done) {
-      this.settings = { filestore: {} }
-      this.requires = {
-        './S3PersistorManager': this.S3PersistorManager,
-        'settings-sharelatex': this.settings,
-        'logger-sharelatex': {
-          log() {},
-          err() {}
-        }
-      }
-      this.PersistorManager = SandboxedModule.require(modulePath, {
-        requires: this.requires
-      })
-      this.PersistorManager.should.respondTo('getFileStream')
-      this.PersistorManager.getFileStream()
-      this.S3PersistorManager.getFileStream.calledOnce.should.equal(true)
-      return done()
-    }))
+  it('should throw an error when the backend is not configured', function() {
+    try {
+      SandboxedModule.require(modulePath, { requires })
+    } catch (err) {
+      expect(err.message).to.equal('no backend specified - config incomplete')
+      return
+    }
+    expect('should have caught an error').not.to.exist
+  })
 
-  return describe('test invalid mixins', () =>
-    it('should not load an invalid wrapper', function(done) {
-      this.settings = {
-        filestore: {
-          backend: 'magic'
-        }
-      }
-      this.requires = {
-        './S3PersistorManager': this.S3PersistorManager,
-        'settings-sharelatex': this.settings,
-        'logger-sharelatex': {
-          log() {},
-          err() {}
-        }
-      }
-      this.fsWrapper = null
-      try {
-        this.PersistorManager = SandboxedModule.require(modulePath, {
-          requires: this.requires
-        })
-      } catch (error) {
-        assert.equal('Unknown filestore backend: magic', error.message)
-      }
-      assert.isNull(this.fsWrapper)
-      return done()
-    }))
+  it('should throw an error when the backend is unknown', function() {
+    settings.filestore.backend = 'magic'
+    try {
+      SandboxedModule.require(modulePath, { requires })
+    } catch (err) {
+      expect(err.message).to.equal('unknown filestore backend: magic')
+      return
+    }
+    expect('should have caught an error').not.to.exist
+  })
 })