openwhisk-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From jamestho...@apache.org
Subject [incubator-openwhisk-runtime-nodejs] branch master updated: Reverts runner refactoring changes. (#141)
Date Fri, 05 Jul 2019 15:53:20 GMT
This is an automated email from the ASF dual-hosted git repository.

jamesthomas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-openwhisk-runtime-nodejs.git


The following commit(s) were added to refs/heads/master by this push:
     new 7c8461c  Reverts runner refactoring changes. (#141)
7c8461c is described below

commit 7c8461c99390aff12e7bad33a2d79f65150b9d03
Author: rodric rabbah <rodric@gmail.com>
AuthorDate: Fri Jul 5 11:53:15 2019 -0400

    Reverts runner refactoring changes. (#141)
    
    * Partially revert runner changes related to code eval.
    
    * Add webpacked test case.
    
    * Simplify test case.
---
 core/nodejsActionBase/runner.js                    | 109 +++++++++------------
 core/nodejsActionBase/src/service.js               |  13 +--
 .../NodeJsActionContainerTests.scala               |  24 +++++
 3 files changed, 77 insertions(+), 69 deletions(-)

diff --git a/core/nodejsActionBase/runner.js b/core/nodejsActionBase/runner.js
index 08bb07e..25cdc6c 100644
--- a/core/nodejsActionBase/runner.js
+++ b/core/nodejsActionBase/runner.js
@@ -23,56 +23,50 @@
 const fs = require('fs');
 const path = require('path');
 
+/** Initializes the handler for the user function. */
+function initializeActionHandler(message) {
+    if (message.binary) {
+        // The code is a base64-encoded zip file.
+        return unzipInTmpDir(message.code)
+            .then(moduleDir => {
+                let parts = splitMainHandler(message.main);
+                if (parts === undefined) {
+                    // message.main is guaranteed to not be empty but be defensive anyway
+                    return Promise.reject('Name of main function is not valid.');
+                }
+
+                // If there is only one property in the "main" handler, it is the function
name
+                // and the module name is specified either from package.json or assumed to
be index.js.
+                let [index, main] = parts;
+
+                // Set the executable directory to the project dir.
+                process.chdir(moduleDir);
+
+                if (index === undefined && !fs.existsSync('package.json') &&
!fs.existsSync('index.js')) {
+                    return Promise.reject('Zipped actions must contain either package.json
or index.js at the root.');
+                }
+
+                //  The module to require.
+                let whatToRequire = index !== undefined ? path.join(moduleDir, index) : moduleDir;
+                let handler = eval('require("' + whatToRequire + '").' + main);
+                return assertMainIsFunction(handler, message.main);
+            })
+            .catch(error => Promise.reject(error));
+    } else try {
+        // The code is a plain old JS file.
+        let handler = eval('(function(){' + message.code + '\nreturn ' + message.main + '})()');
+        return assertMainIsFunction(handler, message.main);
+    } catch (e) {
+        return Promise.reject(e);
+    }
+}
+
 class NodeActionRunner {
 
-    constructor() {
-        this.userScriptMain = undefined;
+    constructor(handler) {
+        this.userScriptMain = handler;
     }
 
-    /** Initializes the runner with the user function. */
-    init(message) {
-        if (message.binary) {
-            // The code is a base64-encoded zip file.
-            return unzipInTmpDir(message.code)
-                .then(moduleDir => {
-                    let parts = splitMainHandler(message.main);
-                    if (parts === undefined) {
-                        // message.main is guaranteed to not be empty but be defensive anyway
-                        return Promise.reject('Name of main function is not valid.');
-                    }
-
-                    // If there is only one property in the "main" handler, it is the function
name
-                    // and the module name is specified either from package.json or assumed
to be index.js.
-                    let [index, main] = parts;
-
-                    // Set the executable directory to the project dir.
-                    process.chdir(moduleDir);
-
-                    if (index === undefined && !fs.existsSync('package.json') &&
!fs.existsSync('index.js')) {
-                        return Promise.reject('Zipped actions must contain either package.json
or index.js at the root.');
-                    }
-
-                    //  The module to require.
-                    let whatToRequire = index !== undefined ? path.join(moduleDir, index)
: moduleDir;
-                    this.userScriptMain = evalScript(main, whatToRequire)
-                    assertMainIsFunction(this.userScriptMain, message.main);
-
-                    // The value 'true' has no special meaning here; the successful state
is
-                    // fully reflected in the successful resolution of the promise.
-                    return true;
-                })
-                .catch(error => Promise.reject(error));
-        } else try {
-            // The code is a plain old JS file.
-            this.userScriptMain = evalScript(message.main, false, message.code)
-            assertMainIsFunction(this.userScriptMain, message.main);
-
-            return Promise.resolve(true); // See comment above about 'true'; it has no specific
meaning.
-        } catch (e) {
-            return Promise.reject(e);
-        }
-    };
-
     run(args) {
         return new Promise((resolve, reject) => {
             try {
@@ -163,22 +157,15 @@ function splitMainHandler(handler) {
     } else return undefined
 }
 
-function assertMainIsFunction(userScriptMain, main) {
-    if (typeof userScriptMain !== 'function') {
-        throw "Action entrypoint '" + main + "' is not a function.";
-    }
-}
-
-/**
- * Evals the code to execute. This is a global function so that the eval is in the global
context
- * and hence functions which use variables without 'var' are permitted.
- */
-function evalScript(main, whatToRequire, code) {
-    if (whatToRequire) {
-        return eval('require("' + whatToRequire + '").' + main);
+function assertMainIsFunction(handler, name) {
+    if (typeof handler === 'function') {
+        return Promise.resolve(handler);
     } else {
-        return eval('(function(){' + code + '\nreturn ' + main + '})()');
+        return Promise.reject("Action entrypoint '" + name + "' is not a function.");
     }
 }
 
-module.exports = NodeActionRunner;
+module.exports = {
+    NodeActionRunner,
+    initializeActionHandler
+};
diff --git a/core/nodejsActionBase/src/service.js b/core/nodejsActionBase/src/service.js
index 11ffeb7..4d994af 100644
--- a/core/nodejsActionBase/src/service.js
+++ b/core/nodejsActionBase/src/service.js
@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-const NodeActionRunner = require('../runner');
+const { initializeActionHandler, NodeActionRunner } = require('../runner');
 
 function NodeActionService(config) {
 
@@ -160,13 +160,10 @@ function NodeActionService(config) {
     };
 
     function doInit(message) {
-        userCodeRunner = new NodeActionRunner();
-
-        return userCodeRunner
-            .init(message)
-            // returning 'true' has no particular meaning here. The fact that the promise
resolved
-            // successfully in itself carries the intended message that initialization succeeded.
-            .then(_ => true)
+        return initializeActionHandler(message)
+            .then(handler => {
+                userCodeRunner = new NodeActionRunner(handler);
+            })
             // emit error to activation log then flush the logs as this is the end of the
activation
             .catch(error => {
                 console.error('Error during initialization:', error);
diff --git a/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala
b/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala
index f677882..63f30fd 100644
--- a/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala
+++ b/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala
@@ -291,6 +291,30 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests
with Ws
     })
   }
 
+  it should "support webpacked function" in {
+    val (out, err) = withNodeJsContainer { c =>
+      val code =
+        """
+          |function foo() {
+          |  return { bar: true }
+          |}
+          |global.main = foo
+        """.stripMargin
+
+      c.init(initPayload(code))._1 should be(200)
+
+      val (runCode, result) = c.run(JsObject.empty)
+      runCode should be(200)
+      result should be(Some(JsObject("bar" -> JsTrue)))
+    }
+
+    checkStreams(out, err, {
+      case (o, e) =>
+        o shouldBe empty
+        e shouldBe empty
+    })
+  }
+
   it should "error when requiring a non-existent package" in {
     // NPM package names cannot start with a dot, and so there is no danger
     // of the package below ever being valid.


Mime
View raw message