Repository: motdotla/node-lambda
Pull request #336 (2017-06-27 02:39:05)

Refactoring `_fileCopy`
- `_fileCopy` now returns Promise
- Also fix `_fileCopy` calling


Patch:
diff --git a/lib/main.js b/lib/main.js
index 3204c7e..bde0a89 100644
--- a/lib/main.js
+++ b/lib/main.js
@@ -97,6 +97,16 @@ Lambda.prototype._runHandler = (handler, event, program, context) => {
     return timeout - (currentTime - startTime)
   }
 
+  // The following three functions are deprecated in AWS Lambda.
+  // Since it is sometimes used by other SDK,
+  // it is a simple one that does not result in `not function` error
+  context.succeed = (result) => console.log(JSON.stringify(result))
+  context.fail = (error) => console.log(JSON.stringify(error))
+  context.done = (error, results) => {
+    console.log(JSON.stringify(error))
+    console.log(JSON.stringify(results))
+  }
+
   handler(event, context, callback)
 }
 
@@ -229,7 +239,7 @@ Lambda.prototype._eventSourceList = function (program) {
   return list
 }
 
-Lambda.prototype._fileCopy = (program, src, dest, excludeNodeModules, callback) => {
+Lambda.prototype._fileCopy = (program, src, dest, excludeNodeModules) => {
   const srcAbsolutePath = path.resolve(src)
   const excludes = (() => {
     return [
@@ -262,34 +272,31 @@ Lambda.prototype._fileCopy = (program, src, dest, excludeNodeModules, callback)
   }).join(',') + '}'
   const dirPatternRegExp = new RegExp(`(${dirBlobs.join('|')})$`)
 
-  fs.mkdirs(dest, (err) => {
-    if (err) {
-      return callback(err)
-    }
-    const options = {
-      dereference: true, // same meaning as `-L` of `rsync` command
-      filter: (src, dest) => {
-        if (!program.prebuiltDirectory && src === path.join(srcAbsolutePath, 'package.json')) {
-          // include package.json unless prebuiltDirectory is set
-          return true
-        }
+  return new Promise((resolve, reject) => {
+    fs.mkdirs(dest, (err) => {
+      if (err) return reject(err)
+      const options = {
+        dereference: true, // same meaning as `-L` of `rsync` command
+        filter: (src, dest) => {
+          if (!program.prebuiltDirectory && src === path.join(srcAbsolutePath, 'package.json')) {
+            // include package.json unless prebuiltDirectory is set
+            return true
+          }
 
-        if (!minimatch(src, pattern, { matchBase: true })) {
-          return true
-        }
-        // Directory check. Even if `src` is a directory it will not end with '/'.
-        if (!dirPatternRegExp.test(src)) {
-          return false
+          if (!minimatch(src, pattern, { matchBase: true })) {
+            return true
+          }
+          // Directory check. Even if `src` is a directory it will not end with '/'.
+          if (!dirPatternRegExp.test(src)) {
+            return false
+          }
+          return !fs.statSync(src).isDirectory()
         }
-        return !fs.statSync(src).isDirectory()
-      }
-    }
-    fs.copy(src, dest, options, function (err) {
-      if (err) {
-        return callback(err)
       }
-
-      return callback(null, true)
+      fs.copy(src, dest, options, (err) => {
+        if (err) return reject(err)
+        resolve()
+      })
     })
   })
 }
@@ -514,13 +521,11 @@ Lambda.prototype._archivePrebuilt = function (program, archiveCallback) {
   const codeDirectory = this._codeDirectory()
   const _this = this
 
-  _this._fileCopy(program, program.prebuiltDirectory, codeDirectory, false, (err) => {
-    if (err) {
-      return archiveCallback(err)
-    }
-
+  _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)
   })
 }
 
@@ -542,29 +547,21 @@ Lambda.prototype._buildAndArchive = function (program, archiveCallback) {
   const codeDirectory = _this._codeDirectory()
   const lambdaSrcDirectory = program.sourceDirectory ? program.sourceDirectory.replace(/\/$/, '') : '.'
 
-  _this._cleanDirectory(codeDirectory).then(() => {
+  return Promise.resolve().then(() => {
+    return _this._cleanDirectory(codeDirectory)
+  }).then(() => {
     console.log('=> Moving files to temporary directory')
-
-    // Move files to tmp folder
-    _this._fileCopy(program, lambdaSrcDirectory, codeDirectory, true, (err) => {
-      if (err) {
-        return archiveCallback(err)
-      }
-
-      console.log('=> Running npm install --production')
-      Promise.resolve().then(() => {
-        return _this._npmInstall(program, codeDirectory)
-      }).then(() => {
-        return _this._postInstallScript(program, codeDirectory)
-      }).then(() => {
-        console.log('=> Zipping deployment package')
-        return _this._zip(program, codeDirectory)
-      }).then((data) => {
-        return archiveCallback(null, data)
-      }).catch((err) => {
-        return archiveCallback(err)
-      })
-    })
+    return _this._fileCopy(program, lambdaSrcDirectory, codeDirectory, true)
+  }).then(() => {
+    console.log('=> Running npm install --production')
+    return _this._npmInstall(program, codeDirectory)
+  }).then(() => {
+    return _this._postInstallScript(program, codeDirectory)
+  }).then(() => {
+    console.log('=> Zipping deployment package')
+    return _this._zip(program, codeDirectory)
+  }).then((data) => {
+    return archiveCallback(null, data)
   }).catch((err) => {
     return archiveCallback(err)
   })


Test patch:
diff --git a/test/main.js b/test/main.js
index ad09bd1..4314d99 100644
--- a/test/main.js
+++ b/test/main.js
@@ -135,6 +135,18 @@ describe('lib/main', function () {
     })
   })
 
+  describe('_runHandler', () => {
+    it('context methods is a function', (done) => {
+      const handler = (event, context, callback) => {
+        assert.isFunction(context.succeed)
+        assert.isFunction(context.fail)
+        assert.isFunction(context.done)
+        done()
+      }
+      lambda._runHandler(handler, {}, program, {})
+    })
+  })
+
   describe('_params', function () {
     // http://docs.aws.amazon.com/lambda/latest/dg/API_CreateFunction.html#SSS-CreateFunction-request-FunctionName
     const functionNamePattern =
@@ -251,16 +263,14 @@ describe('lib/main', function () {
       })
     })
 
-    it('`codeDirectory` is empty. (For `codeDirectory` where the file was present)', (done) => {
-      lambda._fileCopy(program, '.', codeDirectory, true, (err, result) => {
-        assert.isNull(err)
+    it('`codeDirectory` is empty. (For `codeDirectory` where the file was present)', () => {
+      return lambda._fileCopy(program, '.', codeDirectory, true).then(() => {
         const contents = fs.readdirSync(codeDirectory)
         assert.isTrue(contents.length > 0)
-        lambda._cleanDirectory(codeDirectory).then(() => {
+        return lambda._cleanDirectory(codeDirectory).then(() => {
           assert.isTrue(fs.existsSync(codeDirectory))
           const contents = fs.readdirSync(codeDirectory)
           assert.equal(contents.length, 0)
-          done()
         })
       })
     })
@@ -283,9 +293,8 @@ describe('lib/main', function () {
 
     beforeEach(() => lambda._cleanDirectory(codeDirectory))
 
-    it('_fileCopy an index.js as well as other files', (done) => {
-      lambda._fileCopy(program, '.', codeDirectory, true, (err, result) => {
-        assert.isNull(err)
+    it('_fileCopy an index.js as well as other files', () => {
+      return lambda._fileCopy(program, '.', codeDirectory, true).then(() => {
         const contents = fs.readdirSync(codeDirectory);
         ['index.js', 'package.json'].forEach((needle) => {
           assert.include(contents, needle, `Target: "${needle}"`)
@@ -293,7 +302,6 @@ describe('lib/main', function () {
         ['node_modules', 'build'].forEach((needle) => {
           assert.notInclude(contents, needle, `Target: "${needle}"`)
         })
-        done()
       })
     })
 
@@ -311,20 +319,17 @@ describe('lib/main', function () {
         done()
       })
 
-      it('_fileCopy an index.js as well as other files', (done) => {
-        lambda._fileCopy(program, '.', codeDirectory, true, (err, result) => {
-          assert.isNull(err)
+      it('_fileCopy an index.js as well as other files', () => {
+        return lambda._fileCopy(program, '.', codeDirectory, true).then(() => {
           const contents = fs.readdirSync(codeDirectory);
           ['index.js', 'package.json'].forEach((needle) => {
             assert.include(contents, needle, `Target: "${needle}"`)
           })
-          done()
         })
       })
 
-      it('_fileCopy excludes files matching excludeGlobs', (done) => {
-        lambda._fileCopy(program, '.', codeDirectory, true, (err, result) => {
-          assert.isNull(err)
+      it('_fileCopy excludes files matching excludeGlobs', () => {
+        return lambda._fileCopy(program, '.', codeDirectory, true).then(() => {
           let contents = fs.readdirSync(codeDirectory);
           ['__unittest', 'fuga'].forEach((needle) => {
             assert.include(contents, needle, `Target: "${needle}"`)
@@ -343,21 +348,18 @@ describe('lib/main', function () {
 
           contents = fs.readdirSync(path.join(codeDirectory, '__unittest', 'hoge'))
           assert.equal(contents.length, 0, 'directory:__unittest/hoge is empty')
-          done()
         })
       })
 
-      it('_fileCopy should not exclude package.json, even when excluded by excludeGlobs', (done) => {
+      it('_fileCopy should not exclude package.json, even when excluded by excludeGlobs', () => {
         program.excludeGlobs = '*.json'
-        lambda._fileCopy(program, '.', codeDirectory, true, (err, result) => {
-          assert.isNull(err)
+        return lambda._fileCopy(program, '.', codeDirectory, true).then(() => {
           const contents = fs.readdirSync(codeDirectory)
           assert.include(contents, 'package.json')
-          done()
         })
       })
 
-      it('_fileCopy should not include package.json when --prebuiltDirectory is set', (done) => {
+      it('_fileCopy should not include package.json when --prebuiltDirectory is set', () => {
         const buildDir = '.build_' + Date.now()
         after(() => fs.removeSync(buildDir))
 
@@ -367,28 +369,19 @@ describe('lib/main', function () {
 
         program.excludeGlobs = '*.json'
         program.prebuiltDirectory = buildDir
-        lambda._fileCopy(program, buildDir, codeDirectory, true, (err, result) => {
-          assert.isNull(err)
+        return lambda._fileCopy(program, buildDir, codeDirectory, true).then(() => {
           const contents = fs.readdirSync(codeDirectory)
           assert.notInclude(contents, 'package.json', 'Target: "packages.json"')
           assert.include(contents, 'testa', 'Target: "testa"')
-          done()
         })
       })
     })
   })
 
   describe('_npmInstall', () => {
-    beforeEach((done) => {
-      lambda._cleanDirectory(codeDirectory).then(() => {
-        lambda._fileCopy(program, '.', codeDirectory, true, (err) => {
-          if (err) {
-            return done(err)
-          }
-          done()
-        })
-      }).catch((err) => {
-        done(err)
+    beforeEach(() => {
+      return lambda._cleanDirectory(codeDirectory).then(() => {
+        return lambda._fileCopy(program, '.', codeDirectory, true)
       })
     })
 
@@ -403,21 +396,14 @@ describe('lib/main', function () {
   })
 
   describe('_npmInstall (When codeDirectory contains characters to be escaped)', () => {
-    beforeEach((done) => {
+    beforeEach(() => {
       // Since '\' can not be included in the file or directory name in Windows
       const directoryName = process.platform === 'win32'
         ? 'hoge fuga\' piyo'
         : 'hoge "fuga\' \\piyo'
       codeDirectory = path.join(os.tmpdir(), directoryName)
-      lambda._cleanDirectory(codeDirectory).then(() => {
-        lambda._fileCopy(program, '.', codeDirectory, true, (err) => {
-          if (err) {
-            return done(err)
-          }
-          done()
-        })
-      }).catch((err) => {
-        done(err)
+      return lambda._cleanDirectory(codeDirectory).then(() => {
+        return lambda._fileCopy(program, '.', codeDirectory, true)
       })
     })
 
@@ -506,19 +492,14 @@ describe('lib/main', function () {
   })
 
   describe('_zip', () => {
-    beforeEach(function (done) {
+    beforeEach(function () {
       _timeout({ this: this, sec: 30 }) // give it time to build the node modules
-      lambda._cleanDirectory(codeDirectory).then(() => {
-        lambda._fileCopy(program, '.', codeDirectory, true, (err) => {
-          if (err) return done(err)
-          lambda._npmInstall(program, codeDirectory).then(() => {
-            done()
-          }).catch((err) => {
-            done(err)
-          })
-        })
-      }).catch((err) => {
-        done(err)
+      return Promise.resolve().then(() => {
+        return lambda._cleanDirectory(codeDirectory)
+      }).then(() => {
+        return lambda._fileCopy(program, '.', codeDirectory, true)
+      }).then(() => {
+        return lambda._npmInstall(program, codeDirectory)
       })
     })