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 993d086 Expand support for entry point handler to allow module.nested.main. (#132) 993d086 is described below commit 993d0868a8d91e176ae4a8697a85c5612a98967e Author: rodric rabbah <rod...@gmail.com> AuthorDate: Wed Jun 12 11:20:56 2019 -0400 Expand support for entry point handler to allow module.nested.main. (#132) --- core/nodejsActionBase/runner.js | 54 ++- tests/dat/actions/nodejs-test.zip | Bin 2138195 -> 1186 bytes .../NodeJsActionContainerTests.scala | 455 +++++++++++++-------- 3 files changed, 325 insertions(+), 184 deletions(-) diff --git a/core/nodejsActionBase/runner.js b/core/nodejsActionBase/runner.js index ff6fcf3..42cce76 100644 --- a/core/nodejsActionBase/runner.js +++ b/core/nodejsActionBase/runner.js @@ -25,6 +25,7 @@ var util = require('util'); var child_process = require('child_process'); var fs = require('fs'); var path = require('path'); + const serializeError = require('serialize-error'); function NodeActionRunner() { @@ -33,7 +34,7 @@ function NodeActionRunner() { this.userScriptMain = undefined; - this.init = function(message) { + this.init = function (message) { function assertMainIsFunction() { if (typeof thisRunner.userScriptMain !== 'function') { throw "Action entrypoint '" + message.main + "' is not a function."; @@ -44,19 +45,31 @@ function NodeActionRunner() { if (message.binary) { // The code is a base64-encoded zip file. return unzipInTmpDir(message.code).then(function (moduleDir) { - if(!fs.existsSync(path.join(moduleDir, 'package.json')) && - !fs.existsSync(path.join(moduleDir, 'index.js'))) { - return Promise.reject('Zipped actions must contain either package.json or index.js at the root.') + 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; + try { // Set the executable directory to the project dir process.chdir(moduleDir); - thisRunner.userScriptMain = eval('require("' + moduleDir + '").' + message.main); + + 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; + thisRunner.userScriptMain = eval('require("' + whatToRequire + '").' + main); assertMainIsFunction(); - // The value 'true' has no special meaning here; - // the successful state is fully reflected in the - // successful resolution of the promise. + + // The value 'true' has no special meaning here; the successful state is + // fully reflected in the successful resolution of the promise. return true; } catch (e) { return Promise.reject(e); @@ -79,7 +92,7 @@ function NodeActionRunner() { // Returns a Promise with the result of the user code invocation. // The Promise is rejected iff the user code throws. - this.run = function(args) { + this.run = function (args) { return new Promise( function (resolve, reject) { try { @@ -101,9 +114,9 @@ function NodeActionRunner() { // Special case if the user just called `reject()`. if (!error) { - resolve({ error: {}}); + resolve({error: {}}); } else { - resolve({ error: serializeError(error) }); + resolve({error: serializeError(error)}); } }); } @@ -132,9 +145,9 @@ function NodeActionRunner() { }).then(function (zipFile) { return exec(mkTempCmd).then(function (tmpDir2) { return exec("unzip -qq " + zipFile + " -d " + tmpDir2).then(function (res) { - return path.resolve(tmpDir2); + return path.resolve(tmpDir2); }).catch(function (error) { - return Promise.reject("There was an error uncompressing the action archive."); + return Promise.reject("There was an error uncompressing the action archive."); }); }); }); @@ -154,6 +167,21 @@ function NodeActionRunner() { } ); } + + /** + * Splits handler into module name and path to main. + * If the string contains no '.', return [ undefined, the string ]. + * If the string contains one or more '.', return [ string up to first period, rest of the string after ]. + */ + function splitMainHandler(handler) { + let matches = handler.match(/^([^.]+)$|^([^.]+)\.(.+)$/); + if (matches && matches.length == 4) { + let index = matches[2]; + let main = matches[3] || matches[1]; + return [index, main] + } else return undefined + } + } module.exports = NodeActionRunner; diff --git a/tests/dat/actions/nodejs-test.zip b/tests/dat/actions/nodejs-test.zip index a0bfa23..c7483e3 100644 Binary files a/tests/dat/actions/nodejs-test.zip and b/tests/dat/actions/nodejs-test.zip differ diff --git a/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala b/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala index 49c900b..20a5fdb 100644 --- a/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala +++ b/tests/src/test/scala/runtime/actionContainers/NodeJsActionContainerTests.scala @@ -54,54 +54,54 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws override val testEcho = { TestConfig(""" - |function main(args) { - | console.log('hello stdout') - | console.error('hello stderr') - | return args - |} - """.stripMargin) + |function main(args) { + | console.log('hello stdout') + | console.error('hello stderr') + | return args + |} + """.stripMargin) } override val testUnicode = { TestConfig(""" - |function main(args) { - | var str = args.delimiter + " ☃ " + args.delimiter; - | console.log(str); - | return { "winter": str }; - |} - """.stripMargin.trim) + |function main(args) { + | var str = args.delimiter + " ☃ " + args.delimiter; + | console.log(str); + | return { "winter": str }; + |} + """.stripMargin.trim) } override val testEnv = { TestConfig(""" - |function main(args) { - | return { - | "api_host": process.env['__OW_API_HOST'], - | "api_key": process.env['__OW_API_KEY'], - | "namespace": process.env['__OW_NAMESPACE'], - | "action_name": process.env['__OW_ACTION_NAME'], - | "activation_id": process.env['__OW_ACTIVATION_ID'], - | "deadline": process.env['__OW_DEADLINE'] - | } - |} - """.stripMargin.trim) + |function main(args) { + | return { + | "api_host": process.env['__OW_API_HOST'], + | "api_key": process.env['__OW_API_KEY'], + | "namespace": process.env['__OW_NAMESPACE'], + | "action_name": process.env['__OW_ACTION_NAME'], + | "activation_id": process.env['__OW_ACTIVATION_ID'], + | "deadline": process.env['__OW_DEADLINE'] + | } + |} + """.stripMargin.trim) } override val testInitCannotBeCalledMoreThanOnce = { TestConfig(""" - |function main(args) { - | return args - |} - """.stripMargin) + |function main(args) { + | return args + |} + """.stripMargin) } override val testEntryPointOtherThanMain = { TestConfig( """ - | function niam(args) { - | return args; - | } - """.stripMargin, + | function niam(args) { + | return args; + | } + """.stripMargin, main = "niam") } @@ -115,10 +115,11 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "fail to initialize with bad code" in { val (out, err) = withNodeJsContainer { c => - val code = """ - | 10 PRINT "Hello world!" - | 20 GOTO 10 - """.stripMargin + val code = + """ + | 10 PRINT "Hello world!" + | 20 GOTO 10 + """.stripMargin val (initCode, _) = c.init(initPayload(code)) @@ -148,11 +149,12 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "return some error on action error" in { withNodeJsContainer { c => - val code = """ - | function main(args) { - | throw "nooooo"; - | } - """.stripMargin + val code = + """ + | function main(args) { + | throw "nooooo"; + | } + """.stripMargin val (initCode, _) = c.init(initPayload(code)) initCode should be(200) @@ -168,11 +170,12 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "support application errors" in { withNodeJsContainer { c => - val code = """ - | function main(args) { - | return { "error" : "sorry" }; - | } - """.stripMargin; + val code = + """ + | function main(args) { + | return { "error" : "sorry" }; + | } + """.stripMargin; val (initCode, _) = c.init(initPayload(code)) initCode should be(200) @@ -187,18 +190,19 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "support the documentation examples (1)" in { val (out, err) = withNodeJsContainer { c => - val code = """ - | // an action in which each path results in a synchronous activation - | function main(params) { - | if (params.payload == 0) { - | return; - | } else if (params.payload == 1) { - | return {payload: 'Hello, World!'} // indicates normal completion - | } else if (params.payload == 2) { - | return {error: 'payload must be 0 or 1'} // indicates abnormal completion - | } - | } - """.stripMargin + val code = + """ + | // an action in which each path results in a synchronous activation + | function main(params) { + | if (params.payload == 0) { + | return; + | } else if (params.payload == 1) { + | return {payload: 'Hello, World!'} // indicates normal completion + | } else if (params.payload == 2) { + | return {error: 'payload must be 0 or 1'} // indicates abnormal completion + | } + | } + """.stripMargin c.init(initPayload(code))._1 should be(200) @@ -226,21 +230,22 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "support the documentation examples (2)" in { val (out, err) = withNodeJsContainer { c => - val code = """ - | function main(params) { - | if (params.payload) { - | // asynchronous activation - | return new Promise(function(resolve, reject) { - | setTimeout(function() { - | resolve({ done: true }); - | }, 100); - | }) - | } else { - | // synchronous activation - | return {done: true}; - | } - | } - """.stripMargin + val code = + """ + | function main(params) { + | if (params.payload) { + | // asynchronous activation + | return new Promise(function(resolve, reject) { + | setTimeout(function() { + | resolve({ done: true }); + | }, 100); + | }) + | } else { + | // synchronous activation + | return {done: true}; + | } + | } + """.stripMargin c.init(initPayload(code))._1 should be(200) @@ -266,11 +271,12 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws // of the package below ever being valid. // https://docs.npmjs.com/files/package.json val (out, err) = withNodeJsContainer { c => - val code = """ - | function main(args) { - | require('.mildlyinvalidnameofanonexistentpackage'); - | } - """.stripMargin + val code = + """ + | function main(args) { + | require('.mildlyinvalidnameofanonexistentpackage'); + | } + """.stripMargin val (initCode, _) = c.init(initPayload(code)) @@ -290,11 +296,12 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "have openwhisk package available" in { // GIVEN that it should "error when requiring a non-existent package" (see test above for this) val (out, err) = withNodeJsContainer { c => - val code = """ - | function main(args) { - | require('openwhisk'); - | } - """.stripMargin + val code = + """ + | function main(args) { + | require('openwhisk'); + | } + """.stripMargin val (initCode, _) = c.init(initPayload(code)) @@ -316,15 +323,16 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "support resolved promises" in { val (out, err) = withNodeJsContainer { c => - val code = """ - | function main(args) { - | return new Promise(function(resolve, reject) { - | setTimeout(function() { - | resolve({ done: true }); - | }, 100); - | }) - | } - """.stripMargin + val code = + """ + | function main(args) { + | return new Promise(function(resolve, reject) { + | setTimeout(function() { + | resolve({ done: true }); + | }, 100); + | }) + | } + """.stripMargin c.init(initPayload(code))._1 should be(200) @@ -342,15 +350,16 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "support rejected promises" in { val (out, err) = withNodeJsContainer { c => - val code = """ - | function main(args) { - | return new Promise(function(resolve, reject) { - | setTimeout(function() { - | reject({ done: true }); - | }, 100); - | }) - | } - """.stripMargin + val code = + """ + | function main(args) { + | return new Promise(function(resolve, reject) { + | setTimeout(function() { + | reject({ done: true }); + | }, 100); + | }) + | } + """.stripMargin c.init(initPayload(code))._1 should be(200) @@ -369,12 +378,13 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "support rejected promises with no message" in { val (out, err) = withNodeJsContainer { c => - val code = """ - | function main(args) { - | return new Promise(function (resolve, reject) { - | reject(); - | }); - | }""".stripMargin + val code = + """ + | function main(args) { + | return new Promise(function (resolve, reject) { + | reject(); + | }); + | }""".stripMargin c.init(initPayload(code))._1 should be(200) val (runCode, runRes) = c.run(runPayload(JsObject())) @@ -386,14 +396,16 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws val thought = " I took the one less traveled by, and that has made all the difference." val assignment = " x = \"" + thought + "\";\n" - val code = """ - | function main(args) { - | var x = "hello"; - """.stripMargin + (assignment * 7000) + """ - | x = "world"; - | return { "message" : x }; - | } - """.stripMargin + val code = + """ + | function main(args) { + | var x = "hello"; + """.stripMargin + (assignment * 7000) + + """ + | x = "world"; + | return { "message" : x }; + | } + """.stripMargin // Lest someone should make it too easy. code.length should be >= 500000 @@ -414,29 +426,31 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws }) } - val examplePackageDotJson: String = """ - | { - | "name": "wskaction", - | "version": "1.0.0", - | "description": "An OpenWhisk action as an npm package.", - | "main": "index.js", - | "author": "i...@openwhisk.org", - | "license": "Apache-2.0" - | } + val examplePackageDotJson: String = + """ + | { + | "name": "wskaction", + | "version": "1.0.0", + | "description": "An OpenWhisk action as an npm package.", + | "main": "index.js", + | "author": "i...@openwhisk.org", + | "license": "Apache-2.0" + | } """.stripMargin it should "support zip-encoded npm package actions" in { val srcs = Seq( Seq("package.json") -> examplePackageDotJson, - Seq("index.js") -> """ - | exports.main = function (args) { - | var name = typeof args["name"] === "string" ? args["name"] : "stranger"; - | - | return { - | greeting: "Hello " + name + ", from an npm package action." - | }; - | } - """.stripMargin) + Seq("index.js") -> + """ + | exports.main = function (args) { + | var name = typeof args["name"] === "string" ? args["name"] : "stranger"; + | + | return { + | greeting: "Hello " + name + ", from an npm package action." + | }; + | } + """.stripMargin) val code = ZipBuilder.mkBase64Zip(srcs) @@ -458,15 +472,16 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "support zip-encoded npm package actions without a package.json file" in { val srcs = Seq( - Seq("index.js") -> """ - | exports.main = function (args) { - | var name = typeof args["name"] === "string" ? args["name"] : "stranger"; - | - | return { - | greeting: "Hello " + name + ", from an npm package action without a package.json." - | }; - | } - """.stripMargin) + Seq("index.js") -> + """ + | exports.main = function (args) { + | var name = typeof args["name"] === "string" ? args["name"] : "stranger"; + | + | return { + | greeting: "Hello " + name + ", from an npm package action without a package.json." + | }; + | } + """.stripMargin) val code = ZipBuilder.mkBase64Zip(srcs) @@ -487,6 +502,99 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws }) } + it should "support zip-encoded npm package with main from file other than index.js" in { + val srcs = Seq( + Seq("other.js") -> + """ + | exports.niam = function (args) { + | var name = typeof args["name"] === "string" ? args["name"] : "stranger"; + | + | return { + | greeting: "Hello " + name + ", from other.niam." + | }; + | } + """.stripMargin) + + val code = ZipBuilder.mkBase64Zip(srcs) + + val (out, err) = withNodeJsContainer { c => + c.init(initPayload(code, "other.niam"))._1 should be(200) + + val (runCode, runRes) = c.run(runPayload(JsObject())) + runCode should be(200) + runRes.get.fields.get("greeting") shouldBe Some(JsString("Hello stranger, from other.niam.")) + } + + checkStreams(out, err, { + case (o, e) => + o shouldBe empty + e shouldBe empty + }) + } + + it should "support nested main" in { + val srcs = Seq( + Seq("other.js") -> + """ + | exports.niam = { xyz: function (args) { + | var name = typeof args["name"] === "string" ? args["name"] : "stranger"; + | + | return { + | greeting: "Hello " + name + ", from nested main." + | }; + | } } + """.stripMargin) + + val code = ZipBuilder.mkBase64Zip(srcs) + + val (out, err) = withNodeJsContainer { c => + c.init(initPayload(code, "other.niam.xyz"))._1 should be(200) + + val (runCode, runRes) = c.run(runPayload(JsObject())) + runCode should be(200) + runRes.get.fields.get("greeting") shouldBe Some(JsString("Hello stranger, from nested main.")) + } + + checkStreams(out, err, { + case (o, e) => + o shouldBe empty + e shouldBe empty + }) + } + + it should "allow zip-encoded npm package containing package.json and overriding entry point" in { + val srcs = Seq( + Seq("package.json") -> examplePackageDotJson, + Seq("index.js") -> + """ + | exports.main = function (args) { + | return { result: "it works" }; + | } + """, + Seq("other.js") -> + """ + | exports.niam = function (args) { + | return { result: "it should also work" }; + | } + """.stripMargin) + + val code = ZipBuilder.mkBase64Zip(srcs) + + val (out, err) = withNodeJsContainer { c => + c.init(initPayload(code, "other.niam"))._1 should be(200) + + val (runCode, runRes) = c.run(runPayload(JsObject())) + runCode should be(200) + runRes.get.fields.get("result") shouldBe Some(JsString("it should also work")) + } + + checkStreams(out, err, { + case (o, e) => + o shouldBe empty + e shouldBe empty + }) + } + it should "fail gracefully on invalid zip files" in { // Some text-file encoded to base64. val code = "Q2VjaSBuJ2VzdCBwYXMgdW4gemlwLgo=" @@ -504,9 +612,11 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws } it should "fail gracefully on valid zip files that are not actions" in { - val srcs = Seq(Seq("hello") -> """ - | Hello world! - """.stripMargin) + val srcs = Seq( + Seq("hello") -> + """ + | Hello world! + """.stripMargin) val code = ZipBuilder.mkBase64Zip(srcs) @@ -524,11 +634,12 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "support zipped actions using non-default entry point" in { val srcs = Seq( Seq("package.json") -> examplePackageDotJson, - Seq("index.js") -> """ - | exports.niam = function (args) { - | return { result: "it works" }; - | } - """.stripMargin) + Seq("index.js") -> + """ + | exports.niam = function (args) { + | return { result: "it works" }; + | } + """.stripMargin) val code = ZipBuilder.mkBase64Zip(srcs) @@ -544,13 +655,14 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws val srcs = Seq( Seq("package.json") -> examplePackageDotJson, Seq("test.txt") -> "test text", - Seq("index.js") -> s""" - | const fs = require('fs'); - | exports.main = function (args) { - | const fileData = fs.readFileSync('./test.txt').toString(); - | return { result1: fileData, - | result2: __dirname === process.cwd() }; - | } + Seq("index.js") -> + s""" + | const fs = require('fs'); + | exports.main = function (args) { + | const fileData = fs.readFileSync('./test.txt').toString(); + | return { result1: fileData, + | result2: __dirname === process.cwd() }; + | } """.stripMargin) val code = ZipBuilder.mkBase64Zip(srcs) @@ -566,12 +678,13 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws it should "support default function parameters" in { val (out, err) = withNodeJsContainer { c => - val code = """ - | function main(args) { - | let foo = 3; - | return {isValid: (function (a, b = 2) {return a === 3 && b === 2;}(foo))}; - | } - """.stripMargin + val code = + """ + | function main(args) { + | let foo = 3; + | return {isValid: (function (a, b = 2) {return a === 3 && b === 2;}(foo))}; + | } + """.stripMargin val (initCode, _) = c.init(initPayload(code)) initCode should be(200) @@ -596,7 +709,7 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws c.init(initPayload(code))._1 should be(200) val (runCode, runRes) = c.run(runPayload(JsObject())) - runRes.get.fields.get("message") shouldBe Some(JsString("success")) + runRes.get.fields.get("message") shouldBe Some(JsString("hello local library")) } } @@ -604,14 +717,14 @@ abstract class NodeJsActionContainerTests extends BasicActionRunnerTests with Ws withContainer(nodejsTestDockerImageName) { c => val code = """ - | function main(args) { - | var ow = require('openwhisk'); - | // actions only exists on 2.* versions of openwhisk, not 3.*, so if this was 3.* it would throw an error, - | var actions = ow().actions; - | - | return { "message": "success" }; - |} - """.stripMargin + | function main(args) { + | var ow = require('openwhisk'); + | // actions only exists on 2.* versions of openwhisk, not 3.*, so if this was 3.* it would throw an error, + | var actions = ow().actions; + | + | return { "message": "success" }; + |} + """.stripMargin // Initialization of the code should be successful val (initCode, err) = c.init(initPayload(code)) initCode should be(200)