Przeglądaj źródła

Post-decaf cleanup of PersistorManager

Simon Detheridge 6 lat temu
rodzic
commit
dbfacce988

+ 13 - 37
services/filestore/app/js/PersistorManager.js

@@ -1,37 +1,19 @@
-// 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')
-)
+module.exports = (function() {
+  logger.log(
+    {
+      backend: settings.filestore.backend
+    },
+    'Loading backend'
+  )
 
-logger.log(
-  {
-    backend: __guard__(
-      settings != null ? settings.filestore : undefined,
-      x1 => x1.backend
-    )
-  },
-  'Loading backend'
-)
-module.exports = (() => {
-  switch (
-    __guard__(
-      settings != null ? settings.filestore : undefined,
-      x2 => x2.backend
-    )
-  ) {
+  if (!settings.filestore.backend) {
+    throw new Error('no backend specified - config incomplete')
+  }
+
+  switch (settings.filestore.backend) {
     case 'aws-sdk':
       return require('./AWSSDKPersistorManager')
     case 's3':
@@ -40,13 +22,7 @@ module.exports = (() => {
       return require('./FSPersistorManager')
     default:
       throw new Error(
-        `Unknown filestore backend: ${settings.filestore.backend}`
+        `unknown filestore backend: ${settings.filestore.backend}`
       )
   }
 })()
-
-function __guard__(value, transform) {
-  return typeof value !== 'undefined' && value !== null
-    ? transform(value)
-    : undefined
-}

+ 58 - 121
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
+  })
 })