From a5ea679bf99c32fa3c81cea3aa2eabcb3d30b9f8 Mon Sep 17 00:00:00 2001 From: abetomo Date: Wed, 28 Jun 2017 10:21:37 +0900 Subject: [PATCH 1/7] Refactoring _readArchive Modify that _readArchive returns Promise --- lib/main.js | 17 ++++++++++++----- test/main.js | 18 ++++++++---------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/lib/main.js b/lib/main.js index bde0a895..8f867074 100644 --- a/lib/main.js +++ b/lib/main.js @@ -500,17 +500,24 @@ Lambda.prototype._uploadNew = (lambda, params) => { }) } -Lambda.prototype._readArchive = function (program, archiveCallback) { +Lambda.prototype._readArchive = (program) => { if (!fs.existsSync(program.deployZipfile)) { - var err = new Error('No such Zipfile [' + program.deployZipfile + ']') - return archiveCallback(err) + const err = new Error('No such Zipfile [' + program.deployZipfile + ']') + return Promise.reject(err) } - fs.readFile(program.deployZipfile, archiveCallback) + return new Promise((resolve, reject) => { + fs.readFile(program.deployZipfile, (err, data) => { + if (err) return reject(err) + resolve(data) + }) + }) } Lambda.prototype._archive = function (program, archiveCallback) { if (program.deployZipfile && fs.existsSync(program.deployZipfile)) { - return this._readArchive(program, archiveCallback) + return this._readArchive(program) + .then((data) => archiveCallback(null, data)) + .catch((err) => archiveCallback(err)) } return program.prebuiltDirectory ? this._archivePrebuilt(program, archiveCallback) diff --git a/test/main.js b/test/main.js index 4314d992..493562d0 100644 --- a/test/main.js +++ b/test/main.js @@ -590,32 +590,30 @@ describe('lib/main', function () { after(() => fs.unlinkSync(testZipFile)) - it('_readArchive fails (undefined)', (done) => { - lambda._readArchive(program, (err, data) => { + it('_readArchive fails (undefined)', () => { + return lambda._readArchive(program).then((data) => { assert.isUndefined(data) + }).catch((err) => { assert.instanceOf(err, Error) assert.equal(err.message, 'No such Zipfile [undefined]') - done() }) }) - it('_readArchive fails (does not exists file)', (done) => { + it('_readArchive fails (does not exists file)', () => { const filePath = path.join(path.resolve('/aaaa'), 'bbbb') const _program = Object.assign({ deployZipfile: filePath }, program) - lambda._readArchive(_program, (err, data) => { + return lambda._readArchive(_program).then((data) => { assert.isUndefined(data) + }).catch((err) => { assert.instanceOf(err, Error) assert.equal(err.message, `No such Zipfile [${filePath}]`) - done() }) }) - it('_readArchive reads the contents of the zipfile', (done) => { + it('_readArchive reads the contents of the zipfile', () => { const _program = Object.assign({ deployZipfile: testZipFile }, program) - lambda._readArchive(_program, (err, data) => { - assert.isNull(err) + return lambda._readArchive(_program).then((data) => { assert.deepEqual(data, bufferExpected) - done() }) }) From 93ce5a73493fbc57521f74bb3112ecf7a3a3e5b4 Mon Sep 17 00:00:00 2001 From: abetomo Date: Wed, 28 Jun 2017 10:36:16 +0900 Subject: [PATCH 2/7] Refactoring _archivePrebuilt Modify that _archivePrebuilt returns Promise --- lib/main.js | 14 +++++++------- test/main.js | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/main.js b/lib/main.js index 8f867074..bd2b585c 100644 --- a/lib/main.js +++ b/lib/main.js @@ -520,19 +520,19 @@ Lambda.prototype._archive = function (program, archiveCallback) { .catch((err) => archiveCallback(err)) } return program.prebuiltDirectory - ? this._archivePrebuilt(program, archiveCallback) + ? this._archivePrebuilt(program) + .then((data) => archiveCallback(null, data)) + .catch((err) => archiveCallback(err)) : this._buildAndArchive(program, archiveCallback) } -Lambda.prototype._archivePrebuilt = function (program, archiveCallback) { - const codeDirectory = this._codeDirectory() +Lambda.prototype._archivePrebuilt = function (program) { const _this = this + const codeDirectory = _this._codeDirectory() - _this._fileCopy(program, program.prebuiltDirectory, codeDirectory, false).then(() => { + return _this._fileCopy(program, program.prebuiltDirectory, codeDirectory, false).then(() => { console.log('=> Zipping deployment package') - _this._zip(program, codeDirectory).then((data) => archiveCallback(null, data)) - }).catch((err) => { - return archiveCallback(err) + return _this._zip(program, codeDirectory) }) } diff --git a/test/main.js b/test/main.js index 493562d0..ffac874a 100644 --- a/test/main.js +++ b/test/main.js @@ -545,7 +545,7 @@ describe('lib/main', function () { }) }) - it('packages a prebuilt module without installing', function (done) { + it('packages a prebuilt module without installing (It is also a test of `_archivePrebuilt`)', function (done) { _timeout({ this: this, sec: 30 }) // give it time to zip let buildDir = '.build_' + Date.now() after(() => fs.removeSync(buildDir)) From 62df54ba0dcf19713800e8eb086fb7c15e0fbdce Mon Sep 17 00:00:00 2001 From: abetomo Date: Wed, 28 Jun 2017 10:41:12 +0900 Subject: [PATCH 3/7] Refactoring _buildAndArchive --- lib/main.js | 8 +++----- test/main.js | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/lib/main.js b/lib/main.js index bd2b585c..d48265d0 100644 --- a/lib/main.js +++ b/lib/main.js @@ -524,6 +524,8 @@ Lambda.prototype._archive = function (program, archiveCallback) { .then((data) => archiveCallback(null, data)) .catch((err) => archiveCallback(err)) : this._buildAndArchive(program, archiveCallback) + .then((data) => archiveCallback(null, data)) + .catch((err) => archiveCallback(err)) } Lambda.prototype._archivePrebuilt = function (program) { @@ -536,7 +538,7 @@ Lambda.prototype._archivePrebuilt = function (program) { }) } -Lambda.prototype._buildAndArchive = function (program, archiveCallback) { +Lambda.prototype._buildAndArchive = function (program) { if (!fs.existsSync('.env')) { console.warn('[Warning] `.env` file does not exist.') console.info('Execute `node-lambda setup` as necessary and set it up.') @@ -567,10 +569,6 @@ Lambda.prototype._buildAndArchive = function (program, archiveCallback) { }).then(() => { console.log('=> Zipping deployment package') return _this._zip(program, codeDirectory) - }).then((data) => { - return archiveCallback(null, data) - }).catch((err) => { - return archiveCallback(err) }) } diff --git a/test/main.js b/test/main.js index ffac874a..3570c073 100644 --- a/test/main.js +++ b/test/main.js @@ -530,7 +530,7 @@ describe('lib/main', function () { describe('_archive', () => { // archive.files's name is a slash delimiter regardless of platform. - it('installs and zips with an index.js file and node_modules/aws-sdk', function (done) { + it('installs and zips with an index.js file and node_modules/aws-sdk (It is also a test of `_buildAndArchive`)', function (done) { _timeout({ this: this, sec: 30 }) // give it time to zip lambda._archive(program, (err, data) => { From ee1d5efec3b3206fbe9c3c27f87aa2b370f6ed45 Mon Sep 17 00:00:00 2001 From: abetomo Date: Wed, 28 Jun 2017 11:09:46 +0900 Subject: [PATCH 4/7] Refactoring _archive Modify that _archive returns Promise --- lib/main.js | 38 ++++++++++++++------------------------ test/main.js | 24 ++++++++---------------- 2 files changed, 22 insertions(+), 40 deletions(-) diff --git a/lib/main.js b/lib/main.js index d48265d0..ca5ebb86 100644 --- a/lib/main.js +++ b/lib/main.js @@ -513,19 +513,13 @@ Lambda.prototype._readArchive = (program) => { }) } -Lambda.prototype._archive = function (program, archiveCallback) { +Lambda.prototype._archive = function (program) { if (program.deployZipfile && fs.existsSync(program.deployZipfile)) { return this._readArchive(program) - .then((data) => archiveCallback(null, data)) - .catch((err) => archiveCallback(err)) } - return program.prebuiltDirectory - ? this._archivePrebuilt(program) - .then((data) => archiveCallback(null, data)) - .catch((err) => archiveCallback(err)) - : this._buildAndArchive(program, archiveCallback) - .then((data) => archiveCallback(null, data)) - .catch((err) => archiveCallback(err)) + const functionName = program.prebuiltDirectory + ? '_archivePrebuilt' : '_buildAndArchive' + return this[functionName](program) } Lambda.prototype._archivePrebuilt = function (program) { @@ -714,20 +708,18 @@ Lambda.prototype.package = function (program) { } } - _this._archive(program, function (err, buffer) { - if (err) { - throw err - } - - var basename = program.functionName + (program.environment ? '-' + program.environment : '') - var zipfile = path.join(program.packageDirectory, basename + '.zip') + return _this._archive(program).then((buffer) => { + const basename = program.functionName + (program.environment ? '-' + program.environment : '') + const zipfile = path.join(program.packageDirectory, basename + '.zip') console.log('=> Writing packaged zip') - fs.writeFile(zipfile, buffer, function (err) { + fs.writeFile(zipfile, buffer, (err) => { if (err) { throw err } console.log('Packaged zip created: ' + zipfile) }) + }).catch((err) => { + throw err }) } @@ -833,13 +825,11 @@ Lambda.prototype._deployToRegion = function (program, params, region) { Lambda.prototype.deploy = function (program) { const _this = this const regions = program.region.split(',') - _this._archive(program, (err, buffer) => { - if (err) throw err - + return _this._archive(program).then((buffer) => { console.log('=> Reading zip file to memory') const params = _this._params(program, buffer) - Promise.all(regions.map((region) => { + return Promise.all(regions.map((region) => { return _this._deployToRegion(program, params, region) })).then((results) => { const resultsIsEmpty = results.filter((result) => { @@ -851,9 +841,9 @@ Lambda.prototype.deploy = function (program) { console.log('=> All tasks done. Results follow: ') console.log(JSON.stringify(results, null, ' ')) } - }).catch((err) => { - console.log(err) }) + }).catch((err) => { + console.log(err) }) } diff --git a/test/main.js b/test/main.js index 3570c073..dd0ed050 100644 --- a/test/main.js +++ b/test/main.js @@ -530,22 +530,20 @@ describe('lib/main', function () { describe('_archive', () => { // archive.files's name is a slash delimiter regardless of platform. - it('installs and zips with an index.js file and node_modules/aws-sdk (It is also a test of `_buildAndArchive`)', function (done) { + it('installs and zips with an index.js file and node_modules/aws-sdk (It is also a test of `_buildAndArchive`)', function () { _timeout({ this: this, sec: 30 }) // give it time to zip - lambda._archive(program, (err, data) => { - assert.isNull(err) + return lambda._archive(program).then((data) => { const archive = new Zip(data) const contents = Object.keys(archive.files).map((k) => { return archive.files[k].name.toString() }) assert.include(contents, 'index.js') assert.include(contents, 'node_modules/aws-sdk/lib/aws.js') - done() }) }) - it('packages a prebuilt module without installing (It is also a test of `_archivePrebuilt`)', function (done) { + it('packages a prebuilt module without installing (It is also a test of `_archivePrebuilt`)', function () { _timeout({ this: this, sec: 30 }) // give it time to zip let buildDir = '.build_' + Date.now() after(() => fs.removeSync(buildDir)) @@ -558,8 +556,7 @@ describe('lib/main', function () { fs.writeFileSync(path.join(buildDir, 'd', 'testb'), '...') program.prebuiltDirectory = buildDir - lambda._archive(program, (err, data) => { - assert.isNull(err) + return lambda._archive(program).then((data) => { const archive = new Zip(data) const contents = Object.keys(archive.files).map((k) => { return archive.files[k].name.toString() @@ -571,7 +568,6 @@ describe('lib/main', function () { ].forEach((needle) => { assert.include(contents, needle, `Target: "${needle}"`) }) - done() }) }) }) @@ -618,12 +614,11 @@ describe('lib/main', function () { }) describe('If value is set in `deployZipfile`, _readArchive is executed in _archive', () => { - it('`deployZipfile` is a invalid value. Process from creation of zip file', function (done) { + it('`deployZipfile` is a invalid value. Process from creation of zip file', function () { const filePath = path.join(path.resolve('/aaaa'), 'bbbb') const _program = Object.assign({ deployZipfile: filePath }, program) _timeout({ this: this, sec: 30 }) // give it time to zip - lambda._archive(_program, (err, data) => { - assert.isNull(err) + return lambda._archive(_program).then((data) => { // same test as "installs and zips with an index.js file and node_modules/aws-sdk" const archive = new Zip(data) const contents = Object.keys(archive.files).map((k) => { @@ -631,16 +626,13 @@ describe('lib/main', function () { }) assert.include(contents, 'index.js') assert.include(contents, 'node_modules/aws-sdk/lib/aws.js') - done() }) }) - it('`deployZipfile` is a valid value._archive reads the contents of the zipfile', (done) => { + it('`deployZipfile` is a valid value._archive reads the contents of the zipfile', () => { const _program = Object.assign({ deployZipfile: testZipFile }, program) - lambda._archive(_program, (err, data) => { - assert.isNull(err) + return lambda._archive(_program).then((data) => { assert.deepEqual(data, bufferExpected) - done() }) }) }) From 9a38d09e5816b79917b5a2e16050a18d7cc7f870 Mon Sep 17 00:00:00 2001 From: abetomo Date: Wed, 28 Jun 2017 11:10:26 +0900 Subject: [PATCH 5/7] Replace `var` with `const` of `package` --- lib/main.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/main.js b/lib/main.js index ca5ebb86..7f78f96c 100644 --- a/lib/main.js +++ b/lib/main.js @@ -689,12 +689,12 @@ Lambda.prototype._updateScheduleEvents = (scheduleEvents, functionArn, scheduleL } Lambda.prototype.package = function (program) { - var _this = this + const _this = this if (!program.packageDirectory) { throw new Error('packageDirectory not specified!') } try { - var isDir = fs.lstatSync(program.packageDirectory).isDirectory() + const isDir = fs.lstatSync(program.packageDirectory).isDirectory() if (!isDir) { throw new Error(program.packageDirectory + ' is not a directory!') From 76155381414f30b0825370c82fed0e07cc1cb24f Mon Sep 17 00:00:00 2001 From: abetomo Date: Wed, 28 Jun 2017 11:11:46 +0900 Subject: [PATCH 6/7] Remove unnecessary else The condition was reversed. --- lib/main.js | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lib/main.js b/lib/main.js index 7f78f96c..6c8d8a84 100644 --- a/lib/main.js +++ b/lib/main.js @@ -700,12 +700,11 @@ Lambda.prototype.package = function (program) { throw new Error(program.packageDirectory + ' is not a directory!') } } catch (err) { - if (err.code === 'ENOENT') { - console.log('=> Creating package directory') - fs.mkdirsSync(program.packageDirectory) - } else { + if (err.code !== 'ENOENT') { throw err } + console.log('=> Creating package directory') + fs.mkdirsSync(program.packageDirectory) } return _this._archive(program).then((buffer) => { From f6b1d9d7215d2c36755a5248c7e56887ba289f76 Mon Sep 17 00:00:00 2001 From: abetomo Date: Wed, 28 Jun 2017 11:25:32 +0900 Subject: [PATCH 7/7] Revert to the previous writing style That is simpler --- lib/main.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/main.js b/lib/main.js index 6c8d8a84..fccd74ca 100644 --- a/lib/main.js +++ b/lib/main.js @@ -517,9 +517,9 @@ Lambda.prototype._archive = function (program) { if (program.deployZipfile && fs.existsSync(program.deployZipfile)) { return this._readArchive(program) } - const functionName = program.prebuiltDirectory - ? '_archivePrebuilt' : '_buildAndArchive' - return this[functionName](program) + return program.prebuiltDirectory + ? this._archivePrebuilt(program) + : this._buildAndArchive(program) } Lambda.prototype._archivePrebuilt = function (program) {