From 9299487670ec5c3cab811417b634a1d22f28a196 Mon Sep 17 00:00:00 2001
From: hogehoge-banana <tsukasa-chibana@tsukasa-chibana-no-MacBook-Pro.local>
Date: Mon, 24 Aug 2015 17:39:21 +0900
Subject: [PATCH 1/7] use amazon s3 instead of storage-temp

---
 lib/cli.js                    |  63 +++++++++-
 lib/units/storage/amazons3.js | 221 ++++++++++++++++++++++++++++++++++
 package.json                  |   1 +
 3 files changed, 279 insertions(+), 6 deletions(-)
 create mode 100644 lib/units/storage/amazons3.js

diff --git a/lib/cli.js b/lib/cli.js
index 3fc921a5..878faa30 100644
--- a/lib/cli.js
+++ b/lib/cli.js
@@ -731,6 +731,33 @@ program
     })
   })
 
+program
+  .command('storage-s3')
+  .description('start s3 storage')
+  .option('-p, --port <port>'
+    , 'port (or $PORT)'
+    , Number
+    , process.env.PORT || 7100)
+  .option('--bucket <bucketname>'
+    , 'your s3 bucket name'
+    , String)
+  .option('--profile <name>'
+    , 'your aws credentions profile name'
+    , String
+    , 'stf-storage')
+  .option('--endpoint <endpoint>'
+    , 'your buckets endpoint'
+    , String
+    , 's3-ap-northeast-1.amazonaws.com')
+  .action(function(options) {
+    require('./units/storage/amazons3')({
+      port: options.port
+    , profile: options.profile
+    , Bucket: options.bucket
+    , endpoint: options.endpoint
+    })
+  })
+
 program
   .command('storage-plugin-image')
   .description('start storage image plugin')
@@ -942,6 +969,19 @@ program
     , String)
   .option('--mute-master'
     , 'whether to mute master volume when devices are being used')
+  .option('--use-s3'
+    , 'use amazon s3')
+  .option('--s3-bucket <bucketname>'
+    , 'your bucket name'
+    , String)
+  .option('--s3-profile <s3profile>'
+    , 'aws credential Profile name'
+    , String
+    , 'stf-storage')
+  .option('--s3-endpoint <s3endpoint>'
+    , 's3 endpoint'
+    , String
+    , 's3-ap-northeast-1.amazonaws.com')
   .action(function() {
     var log = logger.createLogger('cli:local')
       , args = arguments
@@ -951,7 +991,7 @@ program
     // Each forked process waits for signals to stop, and so we run over the
     // default limit of 10. So, it's not a leak, but a refactor wouldn't hurt.
     process.setMaxListeners(20)
-
+    
     function run() {
       var procs = [
         // app triproxy
@@ -1058,11 +1098,22 @@ program
           ])
 
         // storage
-      , procutil.fork(__filename, [
-            'storage-temp'
-          , '--port', options.storagePort
-          ])
-
+      , (function() {
+	if (!options.useS3) {
+	  return procutil.fork(__filename, [
+              'storage-temp'
+            , '--port', options.storagePort
+            ]);
+	} else {
+	  return procutil.fork(__filename, [
+              'storage-s3'
+            , '--port', options.storagePort
+	    , '--bucket', options.s3Bucket
+	    , '--profile', options.s3Profile
+	    , '--endpoint', options.s3Endpoint
+            ])
+	}
+      })()
         // image processor
       , procutil.fork(__filename, [
             'storage-plugin-image'
diff --git a/lib/units/storage/amazons3.js b/lib/units/storage/amazons3.js
new file mode 100644
index 00000000..48bae43d
--- /dev/null
+++ b/lib/units/storage/amazons3.js
@@ -0,0 +1,221 @@
+var http = require('http')
+var util = require('util')
+var path = require('path')
+
+var express = require('express')
+var validator = require('express-validator')
+var bodyParser = require('body-parser')
+var formidable = require('formidable')
+var Promise = require('bluebird')
+var uuid = require('node-uuid')
+var fs = require('fs')
+
+var logger = require('../../util/logger')
+var Storage = require('../../util/storage')
+var requtil = require('../../util/requtil')
+var download = require('../../util/download')
+
+var AWS = require('aws-sdk')
+var fs = require('fs');
+
+module.exports = function(options) {
+  var log = logger.createLogger('storage:s3')
+    , app = express()
+    , server = http.createServer(app)
+    , credentials = new AWS.SharedIniFileCredentials({
+        profile: options.profile})
+    , bucket = options.Bucket
+    , s3
+
+  log.debug(options)
+
+  AWS.config.credentials = credentials;
+  s3 = new AWS.S3(options)
+
+  app.set('strict routing', true)
+  app.set('case sensitive routing', true)
+  app.set('trust proxy', true)
+
+  app.use(bodyParser.json())
+  app.use(validator())
+
+  function makePutParams(id, path, name) {
+    var key = id + name
+          ? util.format('/%s', name)
+          : ''
+      , rs = fs.createReadStream(path)
+    return {
+      Bucket: bucket
+    , Key: key
+    , Body: rs
+    }
+  }
+
+  app.post('/s/download/:plugin', function(req, res) {
+    requtil.validate(req, function() {
+        req.checkBody('url').notEmpty()
+      })
+    .then(function() {
+      return download(req.body.url, {
+	dir: options.cahedir
+      })
+    })
+    .then(function(file){
+      return new Promise(function(resolve, reject) {
+	var id = uuid.v4()
+        , params = makePutParams(id, file.path)
+	s3.putObject(params, function(err, data) {
+	  if (err)  {
+	    log.err('failed to store "%s"', params.Key);
+	    reject(err)
+	  } else {
+	    resolve({
+	      id: id
+	    , name: file.name
+	    })
+	  }
+	})
+      })
+    })
+    .then(function(file){
+      res.status(201)
+	.json({
+          success: true
+          , resource: {
+	    date: new Date()
+	    , plugin: plugin
+	    , id: file.id
+	    , name: file.name
+	    , href: util.format(
+              '/s/%s/%s%s'
+	      , plugin
+	      , file.id
+	      , file.name
+                ? util.format('/%s', path.basename(file.name))
+                : ''
+	    )
+          }
+        })
+      
+    })
+    .catch(requtil.ValidationError, function(err) {
+      res.status(400)
+        .json({
+          success: false
+          , error: 'ValidationError'
+          , validationErrors: err.errors
+        })
+    })
+    .catch(function(err) {
+      log.error('Error storing resource', err.stack)
+      res.status(500)
+        .json({
+          success: false
+          , error: 'ServerError'
+        })
+    })
+  })
+
+  app.post('/s/upload/:plugin', function(req, res) {
+    var form = new formidable.IncomingForm()
+    var plugin = req.params.plugin
+    Promise.promisify(form.parse, form)(req)
+      .spread(function(fields, files) {
+	var requests = Object.keys(files).map(function(field) {
+          var file = files[field]
+          log.info('Uploaded "%s" to "%s"', file.name, file.path)
+
+	  return new Promise(function(resolve, reject) {
+	    var id = uuid.v4()
+	      , key = id + '/' + file.name
+	      , rs = fs.createReadStream(file.path)
+	      , params = {
+	        Key: key
+		, Body: rs
+		, Bucket: bucket
+		, Metadata: {
+		  field: field
+		  , plugin: plugin
+	        }
+	    };
+	    s3.putObject(params, function(err, data) {
+	      if (err) {
+		log.error('failed to store "%s" bucket:"%s"', 
+			  key, bucket)
+		log.error(err);
+		reject(err);
+	      } else {
+		log.info('Stored "%s" to %s/%s/%s', file.name,
+			bucket, id, file.name)
+		resolve({
+		    field: field
+		  , id: id
+		  , name: file.name
+		});
+	      }
+	    });
+	  });
+        });
+        return Promise.all(requests)
+      })
+      .then(function(storedFiles) {
+	res.status(201)
+	  .json({
+	    success: true
+	  , resources: (function() {
+	      var mapped = Object.create(null)
+	      storedFiles.forEach(function(file) {
+		var plugin = req.params.plugin
+		mapped[file.field] = {
+		    date: new Date()
+		  , plugin: plugin
+		  , id: file.id
+		  , name: file.name
+		  , href: util.format(
+		      '/s/%s/%s%s'
+		    , plugin
+		    , file.id
+		    , file.name
+		        ? util.format('/%s', path.basename(file.name))
+		        : ''
+		  )
+		}
+	      })
+	      return mapped;
+	    })()
+	  })
+      })
+      .catch(function(err) {
+        log.error('Error storing resource', err.stack)
+        res.status(500)
+          .json({
+            success: false
+          , error: 'ServerError'
+          })
+      })
+  });
+
+  app.get('/s/blob/:id/:name', function(req, res) {
+    var path = util.format('%s/%s', req.params.id, req.params.name)
+    var params = {
+         Key: path
+      ,  Bucket: bucket
+    };
+    s3.getObject(params, function(err, data) {
+      if (err)  {
+	log.error('failed to retreive[' + path + ']')
+	log.error(err, err.stack);
+	res.sendStatus(404)
+      } else {
+	res.set({
+	  'Content-type': data.ContentType
+	});
+	res.send(data.Body);
+      }
+    });
+  });
+
+  server.listen(options.port)
+  console.log('Listening on port %d', options.port)
+}
+
diff --git a/package.json b/package.json
index 2743a44d..4f12fe76 100644
--- a/package.json
+++ b/package.json
@@ -81,6 +81,7 @@
   },
   "devDependencies": {
     "async": "^1.4.0",
+    "aws-sdk": "^2.1.46",
     "bower": "^1.3.12",
     "chai": "^3.2.0",
     "css-loader": "^0.14.0",

From 8462290d342f57ab7b9f4b7d32d5c492ef3ab122 Mon Sep 17 00:00:00 2001
From: hogehoge-banana <tsukasa-chibana@tsukasa-chibana-no-MacBook-Pro.local>
Date: Tue, 20 Oct 2015 00:55:33 +0900
Subject: [PATCH 2/7] fix file expiration logic and some bugs.

---
 lib/cli.js                    |  34 ++--
 lib/units/storage/amazons3.js | 294 ++++++++++++++++------------------
 package.json                  |   1 +
 3 files changed, 152 insertions(+), 177 deletions(-)

diff --git a/lib/cli.js b/lib/cli.js
index 878faa30..5212ec0c 100644
--- a/lib/cli.js
+++ b/lib/cli.js
@@ -755,6 +755,7 @@ program
     , profile: options.profile
     , Bucket: options.bucket
     , endpoint: options.endpoint
+    , expires: options.expires
     })
   })
 
@@ -922,10 +923,18 @@ program
     , 'websocket port'
     , Number
     , 7110)
+  .option('--storage-type <temp|s3>'
+    , 'storage type'
+    , String
+    , 'temp')
   .option('--storage-port <port>'
     , 'storage port'
     , Number
     , 7102)
+  .option('--storage-options <json>'
+    , 'array of options to pass to the storage implementation'
+    , String
+    , '[]')
   .option('--storage-plugin-image-port <port>'
     , 'storage image plugin port'
     , Number
@@ -972,7 +981,7 @@ program
   .option('--use-s3'
     , 'use amazon s3')
   .option('--s3-bucket <bucketname>'
-    , 'your bucket name'
+    , 'your Bucket name'
     , String)
   .option('--s3-profile <s3profile>'
     , 'aws credential Profile name'
@@ -991,7 +1000,7 @@ program
     // Each forked process waits for signals to stop, and so we run over the
     // default limit of 10. So, it's not a leak, but a refactor wouldn't hurt.
     process.setMaxListeners(20)
-    
+
     function run() {
       var procs = [
         // app triproxy
@@ -1098,22 +1107,11 @@ program
           ])
 
         // storage
-      , (function() {
-	if (!options.useS3) {
-	  return procutil.fork(__filename, [
-              'storage-temp'
-            , '--port', options.storagePort
-            ]);
-	} else {
-	  return procutil.fork(__filename, [
-              'storage-s3'
-            , '--port', options.storagePort
-	    , '--bucket', options.s3Bucket
-	    , '--profile', options.s3Profile
-	    , '--endpoint', options.s3Endpoint
-            ])
-	}
-      })()
+      ,  procutil.fork(__filename, [
+            util.format('storage-%s', options.storageType)
+          , '--port', options.storagePort
+          ].concat(JSON.parse(options.storageOptions)))
+
         // image processor
       , procutil.fork(__filename, [
             'storage-plugin-image'
diff --git a/lib/units/storage/amazons3.js b/lib/units/storage/amazons3.js
index 48bae43d..d801c228 100644
--- a/lib/units/storage/amazons3.js
+++ b/lib/units/storage/amazons3.js
@@ -10,23 +10,23 @@ var Promise = require('bluebird')
 var uuid = require('node-uuid')
 var fs = require('fs')
 
+var lifecycle = require('../../util/lifecycle')
 var logger = require('../../util/logger')
-var Storage = require('../../util/storage')
 var requtil = require('../../util/requtil')
-var download = require('../../util/download')
 
 var AWS = require('aws-sdk')
-var fs = require('fs');
 
 module.exports = function(options) {
   var log = logger.createLogger('storage:s3')
-    , app = express()
-    , server = http.createServer(app)
-    , credentials = new AWS.SharedIniFileCredentials({
-        profile: options.profile})
-    , bucket = options.Bucket
-    , s3
+  , app = express()
+  , server = http.createServer(app)
+  , credentials = new AWS.SharedIniFileCredentials({
+      profile: options.profile
+    })
+  , bucket = options.Bucket
+  , s3
 
+  // TODO: remove later
   log.debug(options)
 
   AWS.config.credentials = credentials;
@@ -39,151 +39,72 @@ module.exports = function(options) {
   app.use(bodyParser.json())
   app.use(validator())
 
-  function makePutParams(id, path, name) {
-    var key = id + name
-          ? util.format('/%s', name)
-          : ''
-      , rs = fs.createReadStream(path)
-    return {
-      Bucket: bucket
-    , Key: key
-    , Body: rs
-    }
-  }
+  function putObject(plugin, uriorpath, name) {
+    return new Promise(function(resolve, reject) {
+      var id = uuid.v4()
+      var key = id
+      var filename = name? name: path.basename(uriorpath)
+      var rs = fs.createReadStream(uriorpath)
 
+      s3.putObject({
+        Key: key
+      , Body: rs
+      , Bucket: bucket
+      , Metadata: {
+          plugin: plugin
+        , name: filename
+        }
+      }, function(err, data) {
+        if (err) {
+          log.error('failed to store "%s" bucket:"%s"', key, bucket)
+          log.error(err);
+          reject(err);
+        } else {
+          log.info('Stored "%s" to %s/%s', name, bucket, key)
+          resolve({
+            id: id
+          , name: filename
+          })
+        }
+      })
+    })
+  }
+  function getHref(plugin, id, name) {
+    return util.format(
+      '/s/%s/%s%s'
+    , plugin
+    , id
+    , name ? '/' + path.basename(name) : ''
+    )
+  }
   app.post('/s/download/:plugin', function(req, res) {
     requtil.validate(req, function() {
         req.checkBody('url').notEmpty()
       })
-    .then(function() {
-      return download(req.body.url, {
-	dir: options.cahedir
+      .then(function() {
+        return putObject(req.params.plugin, req.body.url)
       })
-    })
-    .then(function(file){
-      return new Promise(function(resolve, reject) {
-	var id = uuid.v4()
-        , params = makePutParams(id, file.path)
-	s3.putObject(params, function(err, data) {
-	  if (err)  {
-	    log.err('failed to store "%s"', params.Key);
-	    reject(err)
-	  } else {
-	    resolve({
-	      id: id
-	    , name: file.name
-	    })
-	  }
-	})
-      })
-    })
-    .then(function(file){
-      res.status(201)
-	.json({
-          success: true
+      .then(function(obj) {
+        var plugin = req.params.plugin
+        res.status(201)
+          .json({
+            success: true
           , resource: {
-	    date: new Date()
-	    , plugin: plugin
-	    , id: file.id
-	    , name: file.name
-	    , href: util.format(
-              '/s/%s/%s%s'
-	      , plugin
-	      , file.id
-	      , file.name
-                ? util.format('/%s', path.basename(file.name))
-                : ''
-	    )
-          }
-        })
-      
-    })
-    .catch(requtil.ValidationError, function(err) {
-      res.status(400)
-        .json({
-          success: false
+              date: new Date()
+            , plugin: plugin
+            , id: obj.id
+            , name: obj.name
+            , href: getHref(plugin, obj.id, obj.name)
+            }
+          })
+      })
+      .catch(requtil.ValidationError, function(err) {
+        res.status(400)
+          .json({
+            success: false
           , error: 'ValidationError'
           , validationErrors: err.errors
-        })
-    })
-    .catch(function(err) {
-      log.error('Error storing resource', err.stack)
-      res.status(500)
-        .json({
-          success: false
-          , error: 'ServerError'
-        })
-    })
-  })
-
-  app.post('/s/upload/:plugin', function(req, res) {
-    var form = new formidable.IncomingForm()
-    var plugin = req.params.plugin
-    Promise.promisify(form.parse, form)(req)
-      .spread(function(fields, files) {
-	var requests = Object.keys(files).map(function(field) {
-          var file = files[field]
-          log.info('Uploaded "%s" to "%s"', file.name, file.path)
-
-	  return new Promise(function(resolve, reject) {
-	    var id = uuid.v4()
-	      , key = id + '/' + file.name
-	      , rs = fs.createReadStream(file.path)
-	      , params = {
-	        Key: key
-		, Body: rs
-		, Bucket: bucket
-		, Metadata: {
-		  field: field
-		  , plugin: plugin
-	        }
-	    };
-	    s3.putObject(params, function(err, data) {
-	      if (err) {
-		log.error('failed to store "%s" bucket:"%s"', 
-			  key, bucket)
-		log.error(err);
-		reject(err);
-	      } else {
-		log.info('Stored "%s" to %s/%s/%s', file.name,
-			bucket, id, file.name)
-		resolve({
-		    field: field
-		  , id: id
-		  , name: file.name
-		});
-	      }
-	    });
-	  });
-        });
-        return Promise.all(requests)
-      })
-      .then(function(storedFiles) {
-	res.status(201)
-	  .json({
-	    success: true
-	  , resources: (function() {
-	      var mapped = Object.create(null)
-	      storedFiles.forEach(function(file) {
-		var plugin = req.params.plugin
-		mapped[file.field] = {
-		    date: new Date()
-		  , plugin: plugin
-		  , id: file.id
-		  , name: file.name
-		  , href: util.format(
-		      '/s/%s/%s%s'
-		    , plugin
-		    , file.id
-		    , file.name
-		        ? util.format('/%s', path.basename(file.name))
-		        : ''
-		  )
-		}
-	      })
-	      return mapped;
-	    })()
-	  })
+          })
       })
       .catch(function(err) {
         log.error('Error storing resource', err.stack)
@@ -193,29 +114,84 @@ module.exports = function(options) {
           , error: 'ServerError'
           })
       })
+  })
+
+  app.post('/s/upload/:plugin', function(req, res, next) {
+    var form = new formidable.IncomingForm()
+    var plugin = req.params.plugin
+    Promise.promisify(form.parse, form)(req)
+      .spread(function(fields, files) {
+        var requests = Object.keys(files).map(function(field) {
+          var file = files[field]
+          log.info('Uploaded "%s" to "%s"', file.name, file.path)
+          return putObject(plugin, file.path, file.name)
+            .then(function (obj) {
+              return {
+                field: field
+              , id: obj.id
+              , name: obj.name
+              , temppath: file.path
+              }
+            })
+        })
+        return Promise.all(requests)
+      })
+      .then(function(storedFiles) {
+        res.status(201).json({
+          success: true,
+          resources: (function() {
+            var mapped = Object.create(null)
+            storedFiles.forEach(function(file) {
+              var plugin = req.params.plugin
+              mapped[file.field] = {
+                date: new Date()
+              , plugin: plugin
+              , id: file.id
+              , name: file.name
+              , href: getHref(plugin, file.id, file.name)
+              }
+            })
+            return mapped;
+          })()
+        })
+        return storedFiles
+      })
+      .then(function (storedFiles){
+        storedFiles.forEach(function (file){
+          log.debug('cleaned up: %s', file.temppath)
+          fs.unlink(file.temppath)
+        })
+      })
+      .catch(function(err) {
+        log.error('Error storing resource', err.stack)
+        res.status(500)
+          .json({
+            success: false,
+            error: 'ServerError'
+          })
+      })
   });
 
   app.get('/s/blob/:id/:name', function(req, res) {
-    var path = util.format('%s/%s', req.params.id, req.params.name)
     var params = {
-         Key: path
-      ,  Bucket: bucket
+      Key: req.params.id,
+      Bucket: bucket
     };
     s3.getObject(params, function(err, data) {
-      if (err)  {
-	log.error('failed to retreive[' + path + ']')
-	log.error(err, err.stack);
-	res.sendStatus(404)
+      if (err) {
+        log.error('failed to retreive[' + path + ']')
+        log.error(err, err.stack);
+        res.sendStatus(404)
       } else {
-	res.set({
-	  'Content-type': data.ContentType
-	});
-	res.send(data.Body);
+        res.set({
+          'Content-type': data.ContentType
+        });
+        res.send(data.Body);
       }
-    });
-  });
+    })
+  })
 
+  // initialize
   server.listen(options.port)
   console.log('Listening on port %d', options.port)
 }
-
diff --git a/package.json b/package.json
index 4f12fe76..20bb3f4c 100644
--- a/package.json
+++ b/package.json
@@ -33,6 +33,7 @@
     "adbkit": "^2.3.1",
     "adbkit-apkreader": "^1.0.0",
     "adbkit-monkey": "^1.0.1",
+    "aws-sdk": "^2.2.3",
     "bluebird": "^2.9.34",
     "body-parser": "^1.13.3",
     "chalk": "~1.0.0",

From d6c0caeb660b8420d3b3f63680810f9197ff88e9 Mon Sep 17 00:00:00 2001
From: tchibana <tsuk.316.banana@gmail.com>
Date: Wed, 21 Oct 2015 23:10:09 +0900
Subject: [PATCH 3/7] fixed cli.js as review comment.

---
 lib/cli.js | 19 +++----------------
 1 file changed, 3 insertions(+), 16 deletions(-)

diff --git a/lib/cli.js b/lib/cli.js
index 404ec3d4..5793fbfd 100644
--- a/lib/cli.js
+++ b/lib/cli.js
@@ -763,7 +763,7 @@ program
     , 'your s3 bucket name'
     , String)
   .option('--profile <name>'
-    , 'your aws credentions profile name'
+    , 'your aws credentials profile name'
     , String
     , 'stf-storage')
   .option('--endpoint <endpoint>'
@@ -774,7 +774,7 @@ program
     require('./units/storage/amazons3')({
       port: options.port
     , profile: options.profile
-    , Bucket: options.bucket
+    , bucket: options.bucket
     , endpoint: options.endpoint
     , expires: options.expires
     })
@@ -1003,19 +1003,6 @@ program
     , [600, 800])
   .option('--mute-master'
     , 'whether to mute master volume when devices are being used')
-  .option('--use-s3'
-    , 'use amazon s3')
-  .option('--s3-bucket <bucketname>'
-    , 'your Bucket name'
-    , String)
-  .option('--s3-profile <s3profile>'
-    , 'aws credential Profile name'
-    , String
-    , 'stf-storage')
-  .option('--s3-endpoint <s3endpoint>'
-    , 's3 endpoint'
-    , String
-    , 's3-ap-northeast-1.amazonaws.com')
   .option('--lock-rotation'
     , 'whether to lock rotation when devices are being used')
   .action(function(serials, options) {
@@ -1134,7 +1121,7 @@ program
           ])
 
         // storage
-      ,  procutil.fork(__filename, [
+      , procutil.fork(__filename, [
             util.format('storage-%s', options.storageType)
           , '--port', options.storagePort
           ].concat(JSON.parse(options.storageOptions)))

From 93a9f921bc483709578e004e3e74b06ca08e3e4c Mon Sep 17 00:00:00 2001
From: tchibana <tsuk.316.banana@gmail.com>
Date: Wed, 21 Oct 2015 23:40:33 +0900
Subject: [PATCH 4/7] fix package.json.

---
 package.json | 1 -
 1 file changed, 1 deletion(-)

diff --git a/package.json b/package.json
index 73931f3d..85d0df50 100644
--- a/package.json
+++ b/package.json
@@ -83,7 +83,6 @@
     "zmq": "^2.13.0"
   },
   "devDependencies": {
-    "aws-sdk": "^2.1.46",
     "async": "^1.4.2",
     "bower": "^1.6.3",
     "chai": "^3.2.0",

From 49dcc7126dc1ab2177a1f86742388a5ee13718fe Mon Sep 17 00:00:00 2001
From: tchibana <tsuk.316.banana@gmail.com>
Date: Wed, 21 Oct 2015 23:45:20 +0900
Subject: [PATCH 5/7] fixed review comments.

---
 lib/units/storage/amazons3.js | 68 +++++++----------------------------
 1 file changed, 13 insertions(+), 55 deletions(-)

diff --git a/lib/units/storage/amazons3.js b/lib/units/storage/amazons3.js
index d801c228..47b891fb 100644
--- a/lib/units/storage/amazons3.js
+++ b/lib/units/storage/amazons3.js
@@ -1,6 +1,7 @@
 var http = require('http')
 var util = require('util')
 var path = require('path')
+var fs = require('fs')
 
 var express = require('express')
 var validator = require('express-validator')
@@ -8,13 +9,12 @@ var bodyParser = require('body-parser')
 var formidable = require('formidable')
 var Promise = require('bluebird')
 var uuid = require('node-uuid')
-var fs = require('fs')
+var AWS = require('aws-sdk')
 
 var lifecycle = require('../../util/lifecycle')
 var logger = require('../../util/logger')
 var requtil = require('../../util/requtil')
 
-var AWS = require('aws-sdk')
 
 module.exports = function(options) {
   var log = logger.createLogger('storage:s3')
@@ -23,14 +23,9 @@ module.exports = function(options) {
   , credentials = new AWS.SharedIniFileCredentials({
       profile: options.profile
     })
-  , bucket = options.Bucket
-  , s3
-
-  // TODO: remove later
-  log.debug(options)
 
   AWS.config.credentials = credentials;
-  s3 = new AWS.S3(options)
+  var s3 = new AWS.S3(options)
 
   app.set('strict routing', true)
   app.set('case sensitive routing', true)
@@ -49,18 +44,18 @@ module.exports = function(options) {
       s3.putObject({
         Key: key
       , Body: rs
-      , Bucket: bucket
+      , Bucket: options.bucket
       , Metadata: {
           plugin: plugin
         , name: filename
         }
       }, function(err, data) {
         if (err) {
-          log.error('failed to store "%s" bucket:"%s"', key, bucket)
+          log.error('failed to store "%s" bucket:"%s"', key, options.bucket)
           log.error(err);
           reject(err);
         } else {
-          log.info('Stored "%s" to %s/%s', name, bucket, key)
+          log.info('Stored "%s" to %s/%s', name, options.bucket, key)
           resolve({
             id: id
           , name: filename
@@ -69,6 +64,7 @@ module.exports = function(options) {
       })
     })
   }
+
   function getHref(plugin, id, name) {
     return util.format(
       '/s/%s/%s%s'
@@ -77,44 +73,6 @@ module.exports = function(options) {
     , name ? '/' + path.basename(name) : ''
     )
   }
-  app.post('/s/download/:plugin', function(req, res) {
-    requtil.validate(req, function() {
-        req.checkBody('url').notEmpty()
-      })
-      .then(function() {
-        return putObject(req.params.plugin, req.body.url)
-      })
-      .then(function(obj) {
-        var plugin = req.params.plugin
-        res.status(201)
-          .json({
-            success: true
-          , resource: {
-              date: new Date()
-            , plugin: plugin
-            , id: obj.id
-            , name: obj.name
-            , href: getHref(plugin, obj.id, obj.name)
-            }
-          })
-      })
-      .catch(requtil.ValidationError, function(err) {
-        res.status(400)
-          .json({
-            success: false
-          , error: 'ValidationError'
-          , validationErrors: err.errors
-          })
-      })
-      .catch(function(err) {
-        log.error('Error storing resource', err.stack)
-        res.status(500)
-          .json({
-            success: false
-          , error: 'ServerError'
-          })
-      })
-  })
 
   app.post('/s/upload/:plugin', function(req, res, next) {
     var form = new formidable.IncomingForm()
@@ -151,7 +109,7 @@ module.exports = function(options) {
               , href: getHref(plugin, file.id, file.name)
               }
             })
-            return mapped;
+            return mapped
           })()
         })
         return storedFiles
@@ -170,13 +128,13 @@ module.exports = function(options) {
             error: 'ServerError'
           })
       })
-  });
+  })
 
   app.get('/s/blob/:id/:name', function(req, res) {
     var params = {
       Key: req.params.id,
-      Bucket: bucket
-    };
+      Bucket: options.bucket
+    }
     s3.getObject(params, function(err, data) {
       if (err) {
         log.error('failed to retreive[' + path + ']')
@@ -185,8 +143,8 @@ module.exports = function(options) {
       } else {
         res.set({
           'Content-type': data.ContentType
-        });
-        res.send(data.Body);
+        })
+        res.send(data.Body)
       }
     })
   })

From e4cb21b1e1e3dbd5206ebdbc61b8f6bdc05b47a8 Mon Sep 17 00:00:00 2001
From: tchibana <tsuk.316.banana@gmail.com>
Date: Thu, 22 Oct 2015 00:22:35 +0900
Subject: [PATCH 6/7] fix review comments

---
 lib/units/storage/amazons3.js | 28 +++++++++++-----------------
 1 file changed, 11 insertions(+), 17 deletions(-)

diff --git a/lib/units/storage/amazons3.js b/lib/units/storage/amazons3.js
index 47b891fb..1bdae6da 100644
--- a/lib/units/storage/amazons3.js
+++ b/lib/units/storage/amazons3.js
@@ -34,32 +34,27 @@ module.exports = function(options) {
   app.use(bodyParser.json())
   app.use(validator())
 
-  function putObject(plugin, uriorpath, name) {
+  function putObject(plugin, file) {
     return new Promise(function(resolve, reject) {
       var id = uuid.v4()
-      var key = id
-      var filename = name? name: path.basename(uriorpath)
-      var rs = fs.createReadStream(uriorpath)
+      var rs = fs.createReadStream(file.path)
 
       s3.putObject({
-        Key: key
+        Key: id
       , Body: rs
       , Bucket: options.bucket
       , Metadata: {
           plugin: plugin
-        , name: filename
+        , name: file.name
         }
       }, function(err, data) {
         if (err) {
-          log.error('failed to store "%s" bucket:"%s"', key, options.bucket)
+          log.error('failed to store "%s" bucket:"%s"', id, options.bucket)
           log.error(err);
           reject(err);
         } else {
-          log.info('Stored "%s" to %s/%s', name, options.bucket, key)
-          resolve({
-            id: id
-          , name: filename
-          })
+          log.info('Stored "%s" to %s/%s', file.name, options.bucket, id)
+          resolve(id)
         }
       })
     })
@@ -82,12 +77,12 @@ module.exports = function(options) {
         var requests = Object.keys(files).map(function(field) {
           var file = files[field]
           log.info('Uploaded "%s" to "%s"', file.name, file.path)
-          return putObject(plugin, file.path, file.name)
-            .then(function (obj) {
+          return putObject(plugin, file)
+            .then(function (id) {
               return {
                 field: field
-              , id: obj.id
-              , name: obj.name
+              , id: id
+              , name: file.name
               , temppath: file.path
               }
             })
@@ -116,7 +111,6 @@ module.exports = function(options) {
       })
       .then(function (storedFiles){
         storedFiles.forEach(function (file){
-          log.debug('cleaned up: %s', file.temppath)
           fs.unlink(file.temppath)
         })
       })

From 467ed01903dedc3ed514aed4a4ec79b7adb8c927 Mon Sep 17 00:00:00 2001
From: Simo Kinnunen <simo@shoqolate.com>
Date: Fri, 6 Nov 2015 02:31:28 +0900
Subject: [PATCH 7/7] Make ESLint happy and use the existing style where
 possible.

---
 lib/cli.js                               | 17 +++--
 lib/units/storage/{amazons3.js => s3.js} | 82 +++++++++++++-----------
 2 files changed, 55 insertions(+), 44 deletions(-)
 rename lib/units/storage/{amazons3.js => s3.js} (68%)

diff --git a/lib/cli.js b/lib/cli.js
index 5793fbfd..6206a73d 100644
--- a/lib/cli.js
+++ b/lib/cli.js
@@ -764,19 +764,24 @@ program
     , String)
   .option('--profile <name>'
     , 'your aws credentials profile name'
-    , String
-    , 'stf-storage')
+    , String)
   .option('--endpoint <endpoint>'
     , 'your buckets endpoint'
-    , String
-    , 's3-ap-northeast-1.amazonaws.com')
+    , String)
   .action(function(options) {
-    require('./units/storage/amazons3')({
+    if (!options.profile) {
+      this.missingArgument('--profile')
+    }
+
+    if (!options.endpoint) {
+      this.missingArgument('--endpoint')
+    }
+
+    require('./units/storage/s3')({
       port: options.port
     , profile: options.profile
     , bucket: options.bucket
     , endpoint: options.endpoint
-    , expires: options.expires
     })
   })
 
diff --git a/lib/units/storage/amazons3.js b/lib/units/storage/s3.js
similarity index 68%
rename from lib/units/storage/amazons3.js
rename to lib/units/storage/s3.js
index 1bdae6da..7581bee5 100644
--- a/lib/units/storage/amazons3.js
+++ b/lib/units/storage/s3.js
@@ -11,21 +11,19 @@ var Promise = require('bluebird')
 var uuid = require('node-uuid')
 var AWS = require('aws-sdk')
 
-var lifecycle = require('../../util/lifecycle')
 var logger = require('../../util/logger')
-var requtil = require('../../util/requtil')
-
 
 module.exports = function(options) {
   var log = logger.createLogger('storage:s3')
-  , app = express()
-  , server = http.createServer(app)
-  , credentials = new AWS.SharedIniFileCredentials({
+    , app = express()
+    , server = http.createServer(app)
+
+  var s3 = new AWS.S3({
+    credentials: new AWS.SharedIniFileCredentials({
       profile: options.profile
     })
-
-  AWS.config.credentials = credentials;
-  var s3 = new AWS.S3(options)
+  , endpoint: options.endpoint
+  })
 
   app.set('strict routing', true)
   app.set('case sensitive routing', true)
@@ -37,23 +35,27 @@ module.exports = function(options) {
   function putObject(plugin, file) {
     return new Promise(function(resolve, reject) {
       var id = uuid.v4()
-      var rs = fs.createReadStream(file.path)
-
       s3.putObject({
         Key: id
-      , Body: rs
+      , Body: fs.createReadStream(file.path)
       , Bucket: options.bucket
       , Metadata: {
           plugin: plugin
         , name: file.name
         }
-      }, function(err, data) {
+      }, function(err) {
         if (err) {
-          log.error('failed to store "%s" bucket:"%s"', id, options.bucket)
-          log.error(err);
-          reject(err);
-        } else {
-          log.info('Stored "%s" to %s/%s', file.name, options.bucket, id)
+          log.error(
+            'Unable to store "%s" as "%s/%s"'
+          , file.temppath
+          , options.bucket
+          , id
+          , err.stack
+          )
+          reject(err)
+        }
+        else {
+          log.info('Stored "%s" as "%s/%s"', file.name, options.bucket, id)
           resolve(id)
         }
       })
@@ -69,7 +71,7 @@ module.exports = function(options) {
     )
   }
 
-  app.post('/s/upload/:plugin', function(req, res, next) {
+  app.post('/s/upload/:plugin', function(req, res) {
     var form = new formidable.IncomingForm()
     var plugin = req.params.plugin
     Promise.promisify(form.parse, form)(req)
@@ -91,11 +93,10 @@ module.exports = function(options) {
       })
       .then(function(storedFiles) {
         res.status(201).json({
-          success: true,
-          resources: (function() {
+          success: true
+        , resources: (function() {
             var mapped = Object.create(null)
             storedFiles.forEach(function(file) {
-              var plugin = req.params.plugin
               mapped[file.field] = {
                 date: new Date()
               , plugin: plugin
@@ -109,41 +110,46 @@ module.exports = function(options) {
         })
         return storedFiles
       })
-      .then(function (storedFiles){
-        storedFiles.forEach(function (file){
-          fs.unlink(file.temppath)
-        })
+      .then(function(storedFiles) {
+        return Promise.all(storedFiles.map(function(file) {
+          return Promise.promisify(fs.unlink, fs)(file.temppath)
+            .catch(function(err) {
+              log.warn('Unable to clean up "%s"', file.temppath, err.stack)
+              return true
+            })
+        }))
       })
       .catch(function(err) {
         log.error('Error storing resource', err.stack)
         res.status(500)
           .json({
-            success: false,
-            error: 'ServerError'
+            success: false
+          , error: 'ServerError'
           })
       })
   })
 
   app.get('/s/blob/:id/:name', function(req, res) {
     var params = {
-      Key: req.params.id,
-      Bucket: options.bucket
+      Key: req.params.id
+    , Bucket: options.bucket
     }
+
     s3.getObject(params, function(err, data) {
       if (err) {
-        log.error('failed to retreive[' + path + ']')
-        log.error(err, err.stack);
+        log.error('Unable to retrieve "%s"', path, err.stack)
         res.sendStatus(404)
-      } else {
-        res.set({
-          'Content-type': data.ContentType
-        })
-        res.send(data.Body)
+        return
       }
+
+      res.set({
+        'Content-Type': data.ContentType
+      })
+
+      res.send(data.Body)
     })
   })
 
-  // initialize
   server.listen(options.port)
   console.log('Listening on port %d', options.port)
 }