Repository: motdotla/node-lambda
Pull request #338 (2017-06-28 02:32:16)

Refactoring `_archive`
I refactored the part related to the _archive function.


Patch:
diff --git a/lib/main.js b/lib/main.js
index bde0a89..fccd74c 100644
--- a/lib/main.js
+++ b/lib/main.js
@@ -500,36 +500,39 @@ 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) {
+Lambda.prototype._archive = function (program) {
   if (program.deployZipfile && fs.existsSync(program.deployZipfile)) {
-    return this._readArchive(program, archiveCallback)
+    return this._readArchive(program)
   }
   return program.prebuiltDirectory
-    ? this._archivePrebuilt(program, archiveCallback)
-    : this._buildAndArchive(program, archiveCallback)
+    ? this._archivePrebuilt(program)
+    : this._buildAndArchive(program)
 }
 
-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)
   })
 }
 
-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.')
@@ -560,10 +563,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)
   })
 }
 
@@ -690,39 +689,36 @@ 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!')
     }
   } 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)
   }
 
-  _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
   })
 }
 
@@ -828,13 +824,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) => {
@@ -846,9 +840,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)
   })
 }
 


Test patch:
diff --git a/test/main.js b/test/main.js
index 4314d99..dd0ed05 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', 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', 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()
       })
     })
   })
@@ -590,42 +586,39 @@ 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()
       })
     })
 
     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) => {
@@ -633,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()
         })
       })
     })