Decaf and promisify ImageOptimiser

services/filestore/app/js/ImageOptimiser.js

@@ -1,44 +1,41 @@
-/* eslint-disable
-    no-unused-vars,
-*/
-// TODO: This file was created by bulk-decaffeinate.
-// Fix any style issues and re-enable lint.
-/*
- * decaffeinate suggestions:
- * DS102: Remove unnecessary code created because of implicit returns
- * DS207: Consider shorter variations of null checks
- * Full docs: https://github.com/decaffeinate/decaffeinate/blob/master/docs/suggestions.md
- */
-const { exec } = require('child_process')
 const logger = require('logger-sharelatex')
-const Settings = require('settings-sharelatex')
+const metrics = require('metrics-sharelatex')
+const { callbackify } = require('util')
+const safeExec = require('./SafeExec').promises
 
 module.exports = {
-  compressPng(localPath, callback) {
+  compressPng: callbackify(compressPng),
-    const startTime = new Date()
+  promises: {
-    logger.log({ localPath }, 'optimising png path')
+    compressPng
-    const args = `optipng ${localPath}`
+  }
-    const opts = {
+}
-      timeout: 30 * 1000,
+
-      killSignal: 'SIGKILL'
+async function compressPng(localPath, callback) {
-    }
+  const timer = new metrics.Timer('compressPng')
-    if (!Settings.enableConversions) {
+  logger.log({ localPath }, 'optimising png path')
-      const error = new Error('Image conversions are disabled')
+
-      return callback(error)
+  const args = ['optipng', localPath]
-    }
+  const opts = {
-    return exec(args, opts, function(err, stdout, stderr) {
+    timeout: 30 * 1000,
-      if (err != null && err.signal === 'SIGKILL') {
+    killSignal: 'SIGKILL'
-        logger.warn({ err, stderr, localPath }, 'optimiser timeout reached')
+  }
-        err = null
+
-      } else if (err != null) {
+  try {
-        logger.err(
+    await safeExec(args, opts)
-          { err, stderr, localPath },
+    timer.done()
-          'something went wrong converting compressPng'
+    logger.log({ localPath }, 'finished compressing png')
-        )
+  } catch (err) {
-      } else {
+    if (err.code === 'SIGKILL') {
-        logger.log({ localPath }, 'finished compressPng file')
+      logger.warn(
-      }
+        { err, stderr: err.stderr, localPath },
-      return callback(err)
+        'optimiser timeout reached'
-    })
+      )
+    } else {
+      logger.err(
+        { err, stderr: err.stderr, localPath },
+        'something went wrong compressing png'
+      )
+      throw err
+    }
   }
 }

services/filestore/test/unit/js/ImageOptimiserTests.js

@@ -1,81 +1,59 @@
-/* eslint-disable
-    handle-callback-err,
-    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 { assert } = require('chai')
 const sinon = require('sinon')
 const chai = require('chai')
-const should = chai.should()
 const { expect } = chai
 const modulePath = '../../../app/js/ImageOptimiser.js'
+const { FailedCommandError } = require('../../../app/js/Errors')
 const SandboxedModule = require('sandboxed-module')
 
 describe('ImageOptimiser', function() {
+  let ImageOptimiser, SafeExec
+  const sourcePath = '/wombat/potato.eps'
+
   beforeEach(function() {
-    this.child_process = { exec: sinon.stub() }
+    SafeExec = {
-    this.settings = { enableConversions: true }
+      promises: sinon.stub().resolves()
-    this.optimiser = SandboxedModule.require(modulePath, {
+    }
+    ImageOptimiser = SandboxedModule.require(modulePath, {
       requires: {
-        child_process: this.child_process,
+        './SafeExec': SafeExec,
         'logger-sharelatex': {
           log() {},
           err() {},
           warn() {}
-        },
+        }
-        'settings-sharelatex': this.settings
       }
     })
-
-    this.sourcePath = '/this/path/here.eps'
-    return (this.error = 'Error')
   })
 
   describe('compressPng', function() {
-    it('convert the file', function(done) {
+    it('should convert the file', function(done) {
-      this.child_process.exec.callsArgWith(2)
+      ImageOptimiser.compressPng(sourcePath, err => {
-      return this.optimiser.compressPng(this.sourcePath, err => {
+        expect(err).not.to.exist
-        const args = this.child_process.exec.args[0][0]
+        expect(SafeExec.promises).to.have.been.calledWith([
-        args.should.equal(`optipng ${this.sourcePath}`)
+          'optipng',
-        return done()
+          sourcePath
+        ])
+        done()
       })
     })
 
-    return it('should return the error', function(done) {
+    it('should return the error', function(done) {
-      this.child_process.exec.callsArgWith(2, this.error)
+      SafeExec.promises.rejects('wombat herding failure')
-      return this.optimiser.compressPng(this.sourcePath, err => {
+      ImageOptimiser.compressPng(sourcePath, err => {
-        err.should.equal(this.error)
+        expect(err.toString()).to.equal('wombat herding failure')
-        return done()
+        done()
       })
     })
   })
 
-  describe('when enableConversions is disabled', () =>
+  describe('when optimiser is sigkilled', function() {
-    it('should produce an error', function(done) {
-      this.settings.enableConversions = false
-      this.child_process.exec.callsArgWith(2)
-      return this.optimiser.compressPng(this.sourcePath, err => {
-        this.child_process.exec.called.should.equal(false)
-        expect(err).to.exist
-        return done()
-      })
-    }))
-
-  return describe('when optimiser is sigkilled', () =>
     it('should not produce an error', function(done) {
-      this.error = new Error('woops')
+      const error = new FailedCommandError('', 'SIGKILL', '', '')
-      this.error.signal = 'SIGKILL'
+      SafeExec.promises.rejects(error)
-      this.child_process.exec.callsArgWith(2, this.error)
+      ImageOptimiser.compressPng(sourcePath, err => {
-      return this.optimiser.compressPng(this.sourcePath, err => {
+        expect(err).not.to.exist
-        expect(err).to.equal(null)
+        done()
-        return done()
       })
-    }))
+    })
+  })
 })