From commits-return-7738-archive-asf-public=cust-asf.ponee.io@openwhisk.apache.org Fri Jul 5 15:53:27 2019 Return-Path: X-Original-To: archive-asf-public@cust-asf.ponee.io Delivered-To: archive-asf-public@cust-asf.ponee.io Received: from mail.apache.org (hermes.apache.org [207.244.88.153]) by mx-eu-01.ponee.io (Postfix) with SMTP id 90F6F180611 for ; Fri, 5 Jul 2019 17:53:26 +0200 (CEST) Received: (qmail 29202 invoked by uid 500); 5 Jul 2019 15:53:26 -0000 Mailing-List: contact commits-help@openwhisk.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: dev@openwhisk.apache.org Delivered-To: mailing list commits@openwhisk.apache.org Received: (qmail 29109 invoked by uid 99); 5 Jul 2019 15:53:25 -0000 Received: from ec2-52-202-80-70.compute-1.amazonaws.com (HELO gitbox.apache.org) (52.202.80.70) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 05 Jul 2019 15:53:25 +0000 Received: by gitbox.apache.org (ASF Mail Server at gitbox.apache.org, from userid 33) id C099387AD7; Fri, 5 Jul 2019 15:53:20 +0000 (UTC) Date: Fri, 05 Jul 2019 15:53:20 +0000 To: "commits@openwhisk.apache.org" Subject: [incubator-openwhisk-runtime-nodejs] branch master updated: Reverts runner refactoring changes. (#141) MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit Message-ID: <156234200071.11190.15933448304360510669@gitbox.apache.org> From: jamesthomas@apache.org X-Git-Host: gitbox.apache.org X-Git-Repo: incubator-openwhisk-runtime-nodejs X-Git-Refname: refs/heads/master X-Git-Reftype: branch X-Git-Oldrev: 16db4bb57d9790f6202c45b2834516cc873a6ff5 X-Git-Newrev: 7c8461c99390aff12e7bad33a2d79f65150b9d03 X-Git-Rev: 7c8461c99390aff12e7bad33a2d79f65150b9d03 X-Git-NotificationType: ref_changed_plus_diff X-Git-Multimail-Version: 1.5.dev Auto-Submitted: auto-generated 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 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.