Refactoring imageRouter to modularity

This should make the imageRouter more modular and easier to extent. Also
a lot of code duplication was removed which should simplify maintenance
in future.

In the new setup we only need to provide a new module file which exports
a function called `uploadImage` and takes a filePath and a callback as
argument. The callback itself takes an error and an url as parameter.
This eliminates the need of a try-catch-block around the statement and
re-enabled the optimization in NodeJS.

Signed-off-by: Sheogorath <sheogorath@shivering-isles.com>

lib/config/index.js

@@ -6,6 +6,7 @@ const path = require('path')
 const {merge} = require('lodash')
 const deepFreeze = require('deep-freeze')
 const {Environment, Permission} = require('./enum')
+const logger = require('../logger')
 
 const appRootPath = path.join(__dirname, '../../')
 const env = process.env.NODE_ENV || Environment.development
@@ -103,6 +104,12 @@ if (config.imageUploadType && !config.imageuploadtype) {
   config.imageuploadtype = config.imageUploadType
 }
 
+// Validate upload upload providers
+if (['filesystem', 's3', 'minio', 'imgur'].indexOf(config.imageuploadtype) === -1) {
+  logger.error('"imageuploadtype" is not correctly set. Please use "filesystem", "s3", "minio" or "imgur". Defaulting to "imgur"')
+  config.imageuploadtype = 'imgur'
+}
+
 // figure out mime types for image uploads
 switch (config.imageuploadtype) {
   case 'imgur':

lib/web/imageRouter.js

@@ -1,132 +0,0 @@
-'use strict'
-var fs = require('fs')
-var url = require('url')
-var path = require('path')
-
-const Router = require('express').Router
-const formidable = require('formidable')
-var imgur = require('imgur')
-
-const config = require('../config')
-const logger = require('../logger')
-const response = require('../response')
-
-const imageRouter = module.exports = Router()
-
-// upload image
-imageRouter.post('/uploadimage', function (req, res) {
-  var form = new formidable.IncomingForm()
-
-  form.keepExtensions = true
-
-  if (config.imageuploadtype === 'filesystem') {
-    form.uploadDir = 'public/uploads'
-  }
-
-  form.parse(req, function (err, fields, files) {
-    if (err || !files.image || !files.image.path) {
-      response.errorForbidden(res)
-    } else {
-      if (config.debug) { logger.info('SERVER received uploadimage: ' + JSON.stringify(files.image)) }
-
-      try {
-        switch (config.imageuploadtype) {
-          case 'filesystem':
-            res.send({
-              link: url.resolve(config.serverurl + '/', files.image.path.match(/^public\/(.+$)/)[1])
-            })
-
-            break
-
-          case 's3':
-            var AWS = require('aws-sdk')
-            var awsConfig = new AWS.Config(config.s3)
-            var s3 = new AWS.S3(awsConfig)
-            const {getImageMimeType} = require('../utils')
-            fs.readFile(files.image.path, function (err, buffer) {
-              if (err) {
-                logger.error(err)
-                res.status(500).end('upload image error')
-                return
-              }
-              var params = {
-                Bucket: config.s3bucket,
-                Key: path.join('uploads', path.basename(files.image.path)),
-                Body: buffer
-              }
-
-              var mimeType = getImageMimeType(files.image.path)
-              if (mimeType) { params.ContentType = mimeType }
-
-              s3.putObject(params, function (err, data) {
-                if (err) {
-                  logger.error(err)
-                  res.status(500).end('upload image error')
-                  return
-                }
-
-                var s3Endpoint = 's3.amazonaws.com'
-                if (config.s3.region && config.s3.region !== 'us-east-1') { s3Endpoint = `s3-${config.s3.region}.amazonaws.com` }
-                res.send({
-                  link: `https://${s3Endpoint}/${config.s3bucket}/${params.Key}`
-                })
-              })
-            })
-            break
-
-          case 'minio':
-            var utils = require('../utils')
-            var Minio = require('minio')
-            var minioClient = new Minio.Client({
-              endPoint: config.minio.endPoint,
-              port: config.minio.port,
-              secure: config.minio.secure,
-              accessKey: config.minio.accessKey,
-              secretKey: config.minio.secretKey
-            })
-            fs.readFile(files.image.path, function (err, buffer) {
-              if (err) {
-                logger.error(err)
-                res.status(500).end('upload image error')
-                return
-              }
-
-              var key = path.join('uploads', path.basename(files.image.path))
-              var protocol = config.minio.secure ? 'https' : 'http'
-
-              minioClient.putObject(config.s3bucket, key, buffer, buffer.size, utils.getImageMimeType(files.image.path), function (err, data) {
-                if (err) {
-                  logger.error(err)
-                  res.status(500).end('upload image error')
-                  return
-                }
-                res.send({
-                  link: `${protocol}://${config.minio.endPoint}:${config.minio.port}/${config.s3bucket}/${key}`
-                })
-              })
-            })
-            break
-
-          case 'imgur':
-          default:
-            imgur.setClientId(config.imgur.clientID)
-            imgur.uploadFile(files.image.path)
-              .then(function (json) {
-                if (config.debug) { logger.info('SERVER uploadimage success: ' + JSON.stringify(json)) }
-                res.send({
-                  link: json.data.link.replace(/^http:\/\//i, 'https://')
-                })
-              })
-              .catch(function (err) {
-                logger.error(err)
-                return res.status(500).end('upload image error')
-              })
-            break
-        }
-      } catch (err) {
-        logger.error(err)
-        return res.status(500).end('upload image error')
-      }
-    }
-  })
-})

lib/web/imageRouter/filesystem.js

@@ -0,0 +1,18 @@
+'use strict'
+const url = require('url')
+
+const config = require('../../config')
+
+exports.uploadImage = function (imagePath, callback) {
+  if (!imagePath || typeof imagePath !== 'string') {
+    callback(new Error('Image path is missing or wrong'), null)
+    return
+  }
+
+  if (!callback || typeof callback !== 'function') {
+    callback(new Error('Callback has to be a function'), null)
+    return
+  }
+
+  callback(null, url.resolve(config.serverurl + '/', imagePath.match(/^public\/(.+$)/)[1]))
+}

lib/web/imageRouter/imgur.js

@@ -0,0 +1,28 @@
+'use strict'
+const config = require('../../config')
+const logger = require('../../logger')
+
+const imgur = require('imgur')
+
+exports.uploadImage = function (imagePath, callback) {
+  if (!imagePath || typeof imagePath !== 'string') {
+    callback(new Error('Image path is missing or wrong'), null)
+    return
+  }
+
+  if (!callback || typeof callback !== 'function') {
+    callback(new Error('Callback has to be a function'), null)
+    return
+  }
+
+  imgur.setClientId(config.imgur.clientID)
+  imgur.uploadFile(imagePath)
+  .then(function (json) {
+    if (config.debug) {
+      logger.info('SERVER uploadimage success: ' + JSON.stringify(json))
+    }
+    callback(null, json.data.link.replace(/^http:\/\//i, 'https://'))
+  }).catch(function (err) {
+    callback(new Error(err), null)
+  })
+}

lib/web/imageRouter/index.js

@@ -0,0 +1,42 @@
+'use strict'
+
+const Router = require('express').Router
+const formidable = require('formidable')
+
+const config = require('../../config')
+const logger = require('../../logger')
+const response = require('../../response')
+
+const imageRouter = module.exports = Router()
+
+// upload image
+imageRouter.post('/uploadimage', function (req, res) {
+  var form = new formidable.IncomingForm()
+
+  form.keepExtensions = true
+
+  if (config.imageuploadtype === 'filesystem') {
+    form.uploadDir = 'public/uploads'
+  }
+
+  form.parse(req, function (err, fields, files) {
+    if (err || !files.image || !files.image.path) {
+      response.errorForbidden(res)
+    } else {
+      if (config.debug) {
+        logger.info('SERVER received uploadimage: ' + JSON.stringify(files.image))
+      }
+
+      const uploadProvider = require('./' + config.imageuploadtype)
+      uploadProvider.uploadImage(files.image.path, function (err, url) {
+        if (err !== null) {
+          logger.error(err)
+          return res.status(500).end('upload image error')
+        }
+        res.send({
+          link: url
+        })
+      })
+    }
+  })
+})

lib/web/imageRouter/minio.js

@@ -0,0 +1,45 @@
+'use strict'
+const fs = require('fs')
+const path = require('path')
+
+const config = require('../../config')
+const {getImageMimeType} = require('../../utils')
+
+const Minio = require('minio')
+const minioClient = new Minio.Client({
+  endPoint: config.minio.endPoint,
+  port: config.minio.port,
+  secure: config.minio.secure,
+  accessKey: config.minio.accessKey,
+  secretKey: config.minio.secretKey
+})
+
+exports.uploadImage = function (imagePath, callback) {
+  if (!imagePath || typeof imagePath !== 'string') {
+    callback(new Error('Image path is missing or wrong'), null)
+    return
+  }
+
+  if (!callback || typeof callback !== 'function') {
+    callback(new Error('Callback has to be a function'), null)
+    return
+  }
+
+  fs.readFile(imagePath, function (err, buffer) {
+    if (err) {
+      callback(new Error(err), null)
+      return
+    }
+
+    let key = path.join('uploads', path.basename(imagePath))
+    let protocol = config.minio.secure ? 'https' : 'http'
+
+    minioClient.putObject(config.s3bucket, key, buffer, buffer.size, getImageMimeType(imagePath), function (err, data) {
+      if (err) {
+        callback(new Error(err), null)
+        return
+      }
+      callback(null, `${protocol}://${config.minio.endPoint}:${config.minio.port}/${config.s3bucket}/${key}`)
+    })
+  })
+}

lib/web/imageRouter/s3.js

@@ -0,0 +1,50 @@
+'use strict'
+const fs = require('fs')
+const path = require('path')
+
+const config = require('../../config')
+const {getImageMimeType} = require('../../utils')
+
+const AWS = require('aws-sdk')
+const awsConfig = new AWS.Config(config.s3)
+const s3 = new AWS.S3(awsConfig)
+
+exports.uploadImage = function (imagePath, callback) {
+  if (!imagePath || typeof imagePath !== 'string') {
+    callback(new Error('Image path is missing or wrong'), null)
+    return
+  }
+
+  if (!callback || typeof callback !== 'function') {
+    callback(new Error('Callback has to be a function'), null)
+    return
+  }
+
+  fs.readFile(imagePath, function (err, buffer) {
+    if (err) {
+      callback(new Error(err), null)
+      return
+    }
+    let params = {
+      Bucket: config.s3bucket,
+      Key: path.join('uploads', path.basename(imagePath)),
+      Body: buffer
+    }
+
+    const mimeType = getImageMimeType(imagePath)
+    if (mimeType) { params.ContentType = mimeType }
+
+    s3.putObject(params, function (err, data) {
+      if (err) {
+        callback(new Error(err), null)
+        return
+      }
+
+      let s3Endpoint = 's3.amazonaws.com'
+      if (config.s3.region && config.s3.region !== 'us-east-1') {
+        s3Endpoint = `s3-${config.s3.region}.amazonaws.com`
+      }
+      callback(null, `https://${s3Endpoint}/${config.s3bucket}/${params.Key}`)
+    })
+  })
+}