From 46f5e37e06571ae841f42898767a4ce0caa850d1 Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 17:00:36 +0900 Subject: [PATCH 01/14] reduce sync operations --- lib/uploadhandler.js | 51 +++++++++++++++++++++++++------------------- 1 file changed, 29 insertions(+), 22 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index ece4b55..8c38c3d 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -4,7 +4,8 @@ var EventEmitter = require('events').EventEmitter, formidable = require('formidable'), imageMagick = require('imagemagick'), mkdirp = require('mkdirp'), - _ = require('lodash'); + _ = require('lodash'), + async = require('async'); module.exports = function (options) { @@ -34,19 +35,25 @@ module.exports = function (options) { this.noCache(); var files = []; fs.readdir(options.uploadDir(), _.bind(function (err, list) { - _.each(list, function (name) { - var stats = fs.statSync(options.uploadDir() + '/' + name), - fileInfo; - if (stats.isFile()) { - fileInfo = new FileInfo({ - name: name, - size: stats.size - }); - this.initUrls(fileInfo); - files.push(fileInfo); - } - }, this); - this.callback({files: files}); + async.each(list, function(name, cb) { + fs.stat(options.uploadDir() + '/' + name, function(err, stats) { + if (!err) { + if (stats.isFile()) { + fileInfo = new FileInfo({ + name: name, + size: stats.size + }); + this.initUrls(fileInfo); + files.push(fileInfo); + } + } + cb(err); + }); + }, + function(err) { + if (err) console.log(err); + this.callback({files: files}); + }); }, this)); }; @@ -87,7 +94,8 @@ module.exports = function (options) { }) .on('file', function (name, file) { var fileInfo = map[path.basename(file.path)]; - if (fs.existsSync(file.path)) { + fs.exists(file.path, function(exists) { + if (exists) { fileInfo.size = file.size; if (!fileInfo.validate()) { fs.unlink(file.path); @@ -98,9 +106,7 @@ module.exports = function (options) { if (options.imageTypes.test(fileInfo.name)) { _.each(options.imageVersions, function (value, version) { // creating directory recursive - if (!fs.existsSync(options.uploadDir() + '/' + version + '/')) - mkdirp.sync(options.uploadDir() + '/' + version + '/'); - + mkdirp(options.uploadDir() + '/' + version + '/', function (err, made) { counter++; var opts = options.imageVersions[version]; imageMagick.resize({ @@ -110,13 +116,12 @@ module.exports = function (options) { dstPath: options.uploadDir() + '/' + version + '/' + fileInfo.name, customArgs: opts.imageArgs || ['-auto-orient'] }, finish); + } }); } } - if (!fs.existsSync(options.uploadDir() + '/')) - mkdirp.sync(options.uploadDir() + '/'); - + mkdirp(options.uploadDir() + '/', function(err, made) { counter++; fs.rename(file.path, options.uploadDir() + '/' + fileInfo.name, function (err) { if (!err) { @@ -127,7 +132,7 @@ module.exports = function (options) { var os = fs.createWriteStream(options.uploadDir() + '/' + fileInfo.name); is.on('end', function (err) { if (!err) { - fs.unlinkSync(file.path); + fs.unlink(file.path); generatePreviews(); } finish(); @@ -135,7 +140,9 @@ module.exports = function (options) { is.pipe(os); } }); + }); } + } }) .on('aborted', function () { _.each(tmpFiles, function (file) { From f36bb8a639cdc46b44b98ba06cfd5b69cb69639e Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 18:22:53 +0900 Subject: [PATCH 02/14] reduce more sync operations --- lib/uploadhandler.js | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index 8c38c3d..044d93f 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -43,8 +43,11 @@ module.exports = function (options) { name: name, size: stats.size }); - this.initUrls(fileInfo); - files.push(fileInfo); + this.initUrls(fileInfo, function(err) { + files.push(fileInfo); + cb(err); + return; + }); } } cb(err); @@ -67,11 +70,15 @@ module.exports = function (options) { redirect, finish = _.bind(function () { if (!--counter) { - _.each(files, function (fileInfo) { - this.initUrls(fileInfo); - this.emit('end', fileInfo); - }, this); - this.callback({files: files}, redirect); + async.each(files, function(fileInfo, cb) { + this.initUrls(fileInfo, function(err) { + this.emit('end', fileInfo); + cb(err); + }); + }, + function(err) { + this.callback({files: files}, redirect); + }); } }, this); @@ -175,15 +182,17 @@ module.exports = function (options) { }); }; - UploadHandler.prototype.initUrls = function (fileInfo) { + UploadHandler.prototype.initUrls = function (fileInfo, cb) { var baseUrl = (options.ssl ? 'https:' : 'http:') + '//' + (options.hostname || this.req.get('Host')); fileInfo.setUrl(null, baseUrl + options.uploadUrl()); fileInfo.setUrl('delete', baseUrl + this.req.originalUrl); - _.each(options.imageVersions, function (value, version) { - if (fs.existsSync(options.uploadDir() + '/' + version + '/' + fileInfo.name)) { - fileInfo.setUrl(version, baseUrl + options.uploadUrl() + '/' + version); - } - }, this); + async.each(Object.keys(options.imageVersions), function(version, cb) { + fs.exists(options.uploadDir() + '/' + version + '/' + fileInfo.name, function(exists) { + if (exists) fileInfo.setUrl(version, baseUrl + options.uploadUrl() + '/' + version); + cb(null); + }) + }, + cb); }; return UploadHandler; From cc105a2924c5463855c5e39a27b12f25f9c81851 Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 18:29:49 +0900 Subject: [PATCH 03/14] binding this to some functions --- lib/uploadhandler.js | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index 044d93f..526d5be 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -35,8 +35,8 @@ module.exports = function (options) { this.noCache(); var files = []; fs.readdir(options.uploadDir(), _.bind(function (err, list) { - async.each(list, function(name, cb) { - fs.stat(options.uploadDir() + '/' + name, function(err, stats) { + async.each(list, _.bind(function(name, cb) { + fs.stat(options.uploadDir() + '/' + name, _.bind(function(err, stats) { if (!err) { if (stats.isFile()) { fileInfo = new FileInfo({ @@ -51,8 +51,8 @@ module.exports = function (options) { } } cb(err); - }); - }, + }, this)); + }, this), function(err) { if (err) console.log(err); this.callback({files: files}); @@ -70,15 +70,15 @@ module.exports = function (options) { redirect, finish = _.bind(function () { if (!--counter) { - async.each(files, function(fileInfo, cb) { - this.initUrls(fileInfo, function(err) { + async.each(files, _.bind(function(fileInfo, cb) { + this.initUrls(fileInfo, _.bind(function(err) { this.emit('end', fileInfo); cb(err); - }); - }, - function(err) { + }, this)); + }, this), + _.bind(function(err) { this.callback({files: files}, redirect); - }); + }, this)); } }, this); From 499475e978b5be15bd529a00877802fe5dc48ca5 Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 19:38:17 +0900 Subject: [PATCH 04/14] fix missing parentheses --- lib/uploadhandler.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index 526d5be..4d9868d 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -123,7 +123,7 @@ module.exports = function (options) { dstPath: options.uploadDir() + '/' + version + '/' + fileInfo.name, customArgs: opts.imageArgs || ['-auto-orient'] }, finish); - } + }); }); } } @@ -149,7 +149,7 @@ module.exports = function (options) { }); }); } - } + }); }) .on('aborted', function () { _.each(tmpFiles, function (file) { From 44087deea96f6b7834b7c2e9d788aa44fec5816b Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 19:39:52 +0900 Subject: [PATCH 05/14] increasing counter for each file before the end event fired --- lib/uploadhandler.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index 4d9868d..12bda0d 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -100,21 +100,23 @@ module.exports = function (options) { } }) .on('file', function (name, file) { + counter++; var fileInfo = map[path.basename(file.path)]; fs.exists(file.path, function(exists) { if (exists) { fileInfo.size = file.size; if (!fileInfo.validate()) { fs.unlink(file.path); + counter--; return; } var generatePreviews = function () { if (options.imageTypes.test(fileInfo.name)) { _.each(options.imageVersions, function (value, version) { + counter++; // creating directory recursive mkdirp(options.uploadDir() + '/' + version + '/', function (err, made) { - counter++; var opts = options.imageVersions[version]; imageMagick.resize({ width: opts.width, @@ -129,7 +131,6 @@ module.exports = function (options) { } mkdirp(options.uploadDir() + '/', function(err, made) { - counter++; fs.rename(file.path, options.uploadDir() + '/' + fileInfo.name, function (err) { if (!err) { generatePreviews(); @@ -149,6 +150,7 @@ module.exports = function (options) { }); }); } + else counter--; }); }) .on('aborted', function () { From 336598a777119e354483869f8cdeb813bf0910df Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 19:47:54 +0900 Subject: [PATCH 06/14] add the async module to the package.json --- package.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/package.json b/package.json index ea657a4..bc17318 100644 --- a/package.json +++ b/package.json @@ -13,7 +13,8 @@ "formidable": ">=1.0.11", "imagemagick": ">=0.1.2", "lodash": ">= 0.9.2", - "mkdirp": ">= 0.3.4" + "mkdirp": ">= 0.3.4", + "async": "*" }, "engines": { "node": ">= 0.8.8" From e36b88a04c7eec0e8753b89f9bc42c6407a6c657 Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 19:53:36 +0900 Subject: [PATCH 07/14] indentation --- lib/uploadhandler.js | 168 +++++++++++++++++++++---------------------- 1 file changed, 84 insertions(+), 84 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index 12bda0d..baf3280 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -5,7 +5,7 @@ var EventEmitter = require('events').EventEmitter, imageMagick = require('imagemagick'), mkdirp = require('mkdirp'), _ = require('lodash'), - async = require('async'); + async = require('async'); module.exports = function (options) { @@ -35,28 +35,28 @@ module.exports = function (options) { this.noCache(); var files = []; fs.readdir(options.uploadDir(), _.bind(function (err, list) { - async.each(list, _.bind(function(name, cb) { - fs.stat(options.uploadDir() + '/' + name, _.bind(function(err, stats) { - if (!err) { - if (stats.isFile()) { - fileInfo = new FileInfo({ - name: name, - size: stats.size - }); - this.initUrls(fileInfo, function(err) { - files.push(fileInfo); - cb(err); - return; - }); - } - } - cb(err); - }, this)); - }, this), - function(err) { - if (err) console.log(err); - this.callback({files: files}); - }); + async.each(list, _.bind(function(name, cb) { + fs.stat(options.uploadDir() + '/' + name, _.bind(function(err, stats) { + if (!err) { + if (stats.isFile()) { + fileInfo = new FileInfo({ + name: name, + size: stats.size + }); + this.initUrls(fileInfo, function(err) { + files.push(fileInfo); + cb(err); + return; + }); + } + } + cb(err); + }, this)); + }, this), + function(err) { + if (err) console.log(err); + this.callback({files: files}); + }); }, this)); }; @@ -70,15 +70,15 @@ module.exports = function (options) { redirect, finish = _.bind(function () { if (!--counter) { - async.each(files, _.bind(function(fileInfo, cb) { + async.each(files, _.bind(function(fileInfo, cb) { this.initUrls(fileInfo, _.bind(function(err) { - this.emit('end', fileInfo); - cb(err); - }, this)); - }, this), - _.bind(function(err) { - this.callback({files: files}, redirect); - }, this)); + this.emit('end', fileInfo); + cb(err); + }, this)); + }, this), + _.bind(function(err) { + this.callback({files: files}, redirect); + }, this)); } }, this); @@ -100,58 +100,58 @@ module.exports = function (options) { } }) .on('file', function (name, file) { - counter++; + counter++; var fileInfo = map[path.basename(file.path)]; - fs.exists(file.path, function(exists) { - if (exists) { - fileInfo.size = file.size; - if (!fileInfo.validate()) { - fs.unlink(file.path); - counter--; - return; - } - - var generatePreviews = function () { - if (options.imageTypes.test(fileInfo.name)) { - _.each(options.imageVersions, function (value, version) { - counter++; - // creating directory recursive - mkdirp(options.uploadDir() + '/' + version + '/', function (err, made) { - var opts = options.imageVersions[version]; - imageMagick.resize({ - width: opts.width, - height: opts.height, - srcPath: options.uploadDir() + '/' + fileInfo.name, - dstPath: options.uploadDir() + '/' + version + '/' + fileInfo.name, - customArgs: opts.imageArgs || ['-auto-orient'] - }, finish); - }); - }); + fs.exists(file.path, function(exists) { + if (exists) { + fileInfo.size = file.size; + if (!fileInfo.validate()) { + fs.unlink(file.path); + counter--; + return; } - } - - mkdirp(options.uploadDir() + '/', function(err, made) { - fs.rename(file.path, options.uploadDir() + '/' + fileInfo.name, function (err) { - if (!err) { - generatePreviews(); - finish(); - } else { - var is = fs.createReadStream(file.path); - var os = fs.createWriteStream(options.uploadDir() + '/' + fileInfo.name); - is.on('end', function (err) { + + var generatePreviews = function () { + if (options.imageTypes.test(fileInfo.name)) { + _.each(options.imageVersions, function (value, version) { + counter++; + // creating directory recursive + mkdirp(options.uploadDir() + '/' + version + '/', function (err, made) { + var opts = options.imageVersions[version]; + imageMagick.resize({ + width: opts.width, + height: opts.height, + srcPath: options.uploadDir() + '/' + fileInfo.name, + dstPath: options.uploadDir() + '/' + version + '/' + fileInfo.name, + customArgs: opts.imageArgs || ['-auto-orient'] + }, finish); + }); + }); + } + } + + mkdirp(options.uploadDir() + '/', function(err, made) { + fs.rename(file.path, options.uploadDir() + '/' + fileInfo.name, function (err) { if (!err) { - fs.unlink(file.path); generatePreviews(); + finish(); + } else { + var is = fs.createReadStream(file.path); + var os = fs.createWriteStream(options.uploadDir() + '/' + fileInfo.name); + is.on('end', function (err) { + if (!err) { + fs.unlink(file.path); + generatePreviews(); + } + finish(); + }); + is.pipe(os); } - finish(); }); - is.pipe(os); - } - }); - }); - } - else counter--; - }); + }); + } + else counter--; + }); }) .on('aborted', function () { _.each(tmpFiles, function (file) { @@ -188,13 +188,13 @@ module.exports = function (options) { var baseUrl = (options.ssl ? 'https:' : 'http:') + '//' + (options.hostname || this.req.get('Host')); fileInfo.setUrl(null, baseUrl + options.uploadUrl()); fileInfo.setUrl('delete', baseUrl + this.req.originalUrl); - async.each(Object.keys(options.imageVersions), function(version, cb) { - fs.exists(options.uploadDir() + '/' + version + '/' + fileInfo.name, function(exists) { - if (exists) fileInfo.setUrl(version, baseUrl + options.uploadUrl() + '/' + version); - cb(null); - }) - }, - cb); + async.each(Object.keys(options.imageVersions), function(version, cb) { + fs.exists(options.uploadDir() + '/' + version + '/' + fileInfo.name, function(exists) { + if (exists) fileInfo.setUrl(version, baseUrl + options.uploadUrl() + '/' + version); + cb(null); + }) + }, + cb); }; return UploadHandler; From b97e921a005cf28202d0e328f1d49655222e401c Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 20:13:40 +0900 Subject: [PATCH 08/14] fix duplicated call of callback --- lib/uploadhandler.js | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index baf3280..e217315 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -37,20 +37,18 @@ module.exports = function (options) { fs.readdir(options.uploadDir(), _.bind(function (err, list) { async.each(list, _.bind(function(name, cb) { fs.stat(options.uploadDir() + '/' + name, _.bind(function(err, stats) { - if (!err) { - if (stats.isFile()) { - fileInfo = new FileInfo({ - name: name, - size: stats.size - }); - this.initUrls(fileInfo, function(err) { - files.push(fileInfo); - cb(err); - return; - }); - } + if (!err && stats.isFile()) { + fileInfo = new FileInfo({ + name: name, + size: stats.size + }); + this.initUrls(fileInfo, function(err) { + files.push(fileInfo); + cb(err); + return; + }); } - cb(err); + else cb(err); }, this)); }, this), function(err) { From f2e62f841f63c1706dd303d9b64c9168a75557d7 Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 20:13:58 +0900 Subject: [PATCH 09/14] fix missing binding this --- lib/uploadhandler.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index e217315..105f27b 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -51,10 +51,10 @@ module.exports = function (options) { else cb(err); }, this)); }, this), - function(err) { + _.bind(function(err) { if (err) console.log(err); this.callback({files: files}); - }); + }, this)); }, this)); }; From 3c8590a2f15253fa89c6da90f96729529b080795 Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 20:15:22 +0900 Subject: [PATCH 10/14] remove unnecessary spaces --- lib/uploadhandler.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index 105f27b..426e212 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -108,7 +108,7 @@ module.exports = function (options) { counter--; return; } - + var generatePreviews = function () { if (options.imageTypes.test(fileInfo.name)) { _.each(options.imageVersions, function (value, version) { @@ -127,7 +127,7 @@ module.exports = function (options) { }); } } - + mkdirp(options.uploadDir() + '/', function(err, made) { fs.rename(file.path, options.uploadDir() + '/' + fileInfo.name, function (err) { if (!err) { From 1965c8ba6ec23572ef6adbfa66dc6f7a5db49ded Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 20:20:54 +0900 Subject: [PATCH 11/14] indent --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index bc17318..eaaa4f5 100644 --- a/package.json +++ b/package.json @@ -14,7 +14,7 @@ "imagemagick": ">=0.1.2", "lodash": ">= 0.9.2", "mkdirp": ">= 0.3.4", - "async": "*" + "async": "*" }, "engines": { "node": ">= 0.8.8" From f28f5e981a4667101b3edafedb92d0289cd1b26d Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 20:37:14 +0900 Subject: [PATCH 12/14] fix missing var. remove unnecessary code --- lib/uploadhandler.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index 426e212..5039b3b 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -38,21 +38,19 @@ module.exports = function (options) { async.each(list, _.bind(function(name, cb) { fs.stat(options.uploadDir() + '/' + name, _.bind(function(err, stats) { if (!err && stats.isFile()) { - fileInfo = new FileInfo({ + var fileInfo = new FileInfo({ name: name, size: stats.size }); this.initUrls(fileInfo, function(err) { files.push(fileInfo); cb(err); - return; }); } else cb(err); }, this)); }, this), _.bind(function(err) { - if (err) console.log(err); this.callback({files: files}); }, this)); }, this)); From 80be7efe227a41929549abf48a9bfbc2c0a3b603 Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 20:38:28 +0900 Subject: [PATCH 13/14] change decrementing counter to calling finish --- lib/uploadhandler.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index 5039b3b..178859b 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -103,7 +103,7 @@ module.exports = function (options) { fileInfo.size = file.size; if (!fileInfo.validate()) { fs.unlink(file.path); - counter--; + finish(); return; } @@ -146,7 +146,7 @@ module.exports = function (options) { }); }); } - else counter--; + else finish(); }); }) .on('aborted', function () { From 3135dec0d47a71d8f5c64abb0358b7851ec1c3c0 Mon Sep 17 00:00:00 2001 From: peecky Date: Fri, 28 Mar 2014 20:48:35 +0900 Subject: [PATCH 14/14] indent --- lib/uploadhandler.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/uploadhandler.js b/lib/uploadhandler.js index 178859b..8403595 100644 --- a/lib/uploadhandler.js +++ b/lib/uploadhandler.js @@ -113,14 +113,14 @@ module.exports = function (options) { counter++; // creating directory recursive mkdirp(options.uploadDir() + '/' + version + '/', function (err, made) { - var opts = options.imageVersions[version]; - imageMagick.resize({ - width: opts.width, - height: opts.height, - srcPath: options.uploadDir() + '/' + fileInfo.name, - dstPath: options.uploadDir() + '/' + version + '/' + fileInfo.name, - customArgs: opts.imageArgs || ['-auto-orient'] - }, finish); + var opts = options.imageVersions[version]; + imageMagick.resize({ + width: opts.width, + height: opts.height, + srcPath: options.uploadDir() + '/' + fileInfo.name, + dstPath: options.uploadDir() + '/' + version + '/' + fileInfo.name, + customArgs: opts.imageArgs || ['-auto-orient'] + }, finish); }); }); }