From 51a0b68d8e26cc629c5b05e6ea283253e42c5015 Mon Sep 17 00:00:00 2001 From: Nick O'Leary Date: Thu, 22 Jun 2023 10:17:48 +0100 Subject: [PATCH] Revert "Add callback to getSetting to support async jsonata access" --- .../src/types/node-red/func.d.ts | 17 --- .../nodes/core/function/10-function.js | 4 +- .../@node-red/runtime/lib/flows/Flow.js | 139 ++++++------------ .../@node-red/runtime/lib/flows/index.js | 2 +- .../@node-red/runtime/lib/flows/util.js | 30 +--- .../node_modules/@node-red/util/lib/util.js | 83 +++-------- .../core/common/91-global-config_spec.js | 26 ---- .../@node-red/runtime/lib/flows/Flow_spec.js | 74 +++------- .../lib/storage/localfilesystem/index_spec.js | 2 +- 9 files changed, 90 insertions(+), 287 deletions(-) diff --git a/packages/node_modules/@node-red/editor-client/src/types/node-red/func.d.ts b/packages/node_modules/@node-red/editor-client/src/types/node-red/func.d.ts index 8544439d9..59f8a8bd7 100644 --- a/packages/node_modules/@node-red/editor-client/src/types/node-red/func.d.ts +++ b/packages/node_modules/@node-red/editor-client/src/types/node-red/func.d.ts @@ -281,21 +281,4 @@ declare class env { * ```const flowName = env.get("NR_FLOW_NAME");``` */ static get(name:string) :any; - /** - * Get an environment variable value (asynchronous). - * - * Predefined node-red variables... - * * `NR_NODE_ID` - the ID of the node - * * `NR_NODE_NAME` - the Name of the node - * * `NR_NODE_PATH` - the Path of the node - * * `NR_GROUP_ID` - the ID of the containing group - * * `NR_GROUP_NAME` - the Name of the containing group - * * `NR_FLOW_ID` - the ID of the flow the node is on - * * `NR_FLOW_NAME` - the Name of the flow the node is on - * @param name Name of the environment variable to get - * @param callback Callback function (`(err,value) => {}`) - * @example - * ```const flowName = env.get("NR_FLOW_NAME");``` - */ - static get(name:string, callback: Function) :void; } diff --git a/packages/node_modules/@node-red/nodes/core/function/10-function.js b/packages/node_modules/@node-red/nodes/core/function/10-function.js index a6ede375f..662b7fa47 100644 --- a/packages/node_modules/@node-red/nodes/core/function/10-function.js +++ b/packages/node_modules/@node-red/nodes/core/function/10-function.js @@ -242,8 +242,8 @@ module.exports = function(RED) { } }, env: { - get: function(envVar, callback) { - return RED.util.getSetting(node, envVar, node._flow, callback); + get: function(envVar) { + return RED.util.getSetting(node, envVar); } }, setTimeout: function () { diff --git a/packages/node_modules/@node-red/runtime/lib/flows/Flow.js b/packages/node_modules/@node-red/runtime/lib/flows/Flow.js index 2e1e97c4b..489ec42cb 100644 --- a/packages/node_modules/@node-red/runtime/lib/flows/Flow.js +++ b/packages/node_modules/@node-red/runtime/lib/flows/Flow.js @@ -416,50 +416,23 @@ class Flow { return this.activeNodes; } - - /** - * Group callback signature - * - * @callback GroupEnvCallback - * @param {Error} err The error object (or null) - * @param {[result: {val:Any}, name: String]} result The result of the callback - * @returns {void} + /*! + * Get value of environment variable defined in group node. + * @param {String} group - group node + * @param {String} name - name of variable + * @return {Object} object containing the value in val property or null if not defined */ - - /** - * @function getGroupEnvSetting - * Get a group setting value synchronously. - * This currently automatically defers to the parent - * @overload - * @param {Object} node - * @param {Object} group - * @param {String} name - * @returns {Any} - * - * Get a group setting value asynchronously. - * @overload - * @param {Object} node - * @param {Object} group - * @param {String} name - * @param {GroupEnvCallback} callback - * @returns {void} - */ - - getGroupEnvSetting(node, group, name, callback) { - /** @type {GroupEnvCallback} */ - const returnOrCallback = (err, [result, newName]) => { - if (callback) { - callback(err, [result, newName]); - return - } - return [result, newName]; - } + getGroupEnvSetting(node, group, name) { if (group) { if (name === "NR_GROUP_NAME") { - return returnOrCallback(null, [{ val: group.name }, null]); + return [{ + val: group.name + }, null]; } if (name === "NR_GROUP_ID") { - return returnOrCallback(null, [{ val: group.id }, null]); + return [{ + val: group.id + }, null]; } if (group.credentials === undefined) { @@ -484,32 +457,33 @@ class Flow { if (env) { let value = env.value; const type = env.type; - if ((type !== "env") || (value !== name)) { + if ((type !== "env") || + (value !== name)) { if (type === "env") { value = value.replace(new RegExp("\\${"+name+"}","g"),"${$parent."+name+"}"); - } else if (type === "bool") { - const val = ((value === "true") || (value === true)); - return returnOrCallback(null, [{ val: val }, null]) + } + if (type === "bool") { + const val + = ((value === "true") || + (value === true)); + return [{ + val: val + }, null]; } if (type === "cred") { - return returnOrCallback(null, [{ val: value }, null]) + return [{ + val: value + }, null]; } try { - if (!callback) { - var val = redUtil.evaluateNodeProperty(value, type, node, null, null); - return [{ val: val }, null]; - } else { - redUtil.evaluateNodeProperty(value, type, node, null, (err, value) => { - return returnOrCallback(err, [{ val: value }, null]) - }); - return - } + var val = redUtil.evaluateNodeProperty(value, type, node, null, null); + return [{ + val: val + }, null]; } catch (e) { - if (!callback) { - this.error(e); - } - return returnOrCallback(e, null); + this.error(e); + return [null, null]; } } } @@ -520,47 +494,27 @@ class Flow { } if (group.g) { const parent = this.getGroupNode(group.g); - const gVal = this.getGroupEnvSetting(node, parent, name, callback); - if (callback) { - return; - } - return gVal; + return this.getGroupEnvSetting(node, parent, name); } } - return returnOrCallback(null, [null, name]); + return [null, name]; } - /** - * Settings callback signature - * - * @callback SettingsCallback - * @param {Error} err The error object (or null) - * @param {Any} result The result of the callback - * @returns {void} - */ + /** * Get a flow setting value. This currently automatically defers to the parent * flow which, as defined in ./index.js returns `process.env[key]`. * This lays the groundwork for Subflow to have instance-specific settings - * @param {String} key The settings key - * @param {SettingsCallback} callback Optional callback function - * @return {Any} + * @param {[type]} key [description] + * @return {[type]} [description] */ - getSetting(key, callback) { - /** @type {SettingsCallback} */ - const returnOrCallback = (err, result) => { - if (callback) { - callback(err, result); - return - } - return result; - } + getSetting(key) { const flow = this.flow; if (key === "NR_FLOW_NAME") { - return returnOrCallback(null, flow.label); + return flow.label; } if (key === "NR_FLOW_ID") { - return returnOrCallback(null, flow.id); + return flow.id; } if (flow.credentials === undefined) { flow.credentials = credentials.get(flow.id) || {}; @@ -590,14 +544,15 @@ class Flow { } try { if (type === "bool") { - const val = ((value === "true") || (value === true)); - return returnOrCallback(null, val); + const val = ((value === "true") || + (value === true)); + return val; } if (type === "cred") { - return returnOrCallback(null, value); + return value; } var val = redUtil.evaluateNodeProperty(value, type, null, null, null); - return returnOrCallback(null, val); + return val; } catch (e) { this.error(e); @@ -609,11 +564,7 @@ class Flow { key = key.substring(8); } } - const pVal = this.parent.getSetting(key, callback); - if (callback) { - return; - } - return pVal; + return this.parent.getSetting(key); } /** diff --git a/packages/node_modules/@node-red/runtime/lib/flows/index.js b/packages/node_modules/@node-red/runtime/lib/flows/index.js index 9c6133ea5..e18861f17 100644 --- a/packages/node_modules/@node-red/runtime/lib/flows/index.js +++ b/packages/node_modules/@node-red/runtime/lib/flows/index.js @@ -780,7 +780,7 @@ const flowAPI = { getNode: getNode, handleError: () => false, handleStatus: () => false, - getSetting: (k, callback) => flowUtil.getEnvVar(k, callback), + getSetting: k => flowUtil.getEnvVar(k), log: m => log.log(m) } diff --git a/packages/node_modules/@node-red/runtime/lib/flows/util.js b/packages/node_modules/@node-red/runtime/lib/flows/util.js index 39ff820b4..868657c7b 100644 --- a/packages/node_modules/@node-red/runtime/lib/flows/util.js +++ b/packages/node_modules/@node-red/runtime/lib/flows/util.js @@ -308,34 +308,16 @@ module.exports = { runtime.settings.envVarExcludes.forEach(v => envVarExcludes[v] = true); } }, - /** - * Get the value of an environment variable - * Call with a callback to get the value asynchronously - * or without to get the value synchronously - * @param {String} key The name of the environment variable - * @param {(err: Error, val: Any)} [callback] Optional callback for asynchronous call - * @returns {Any | void} The value of the environment variable or undefined if not found - */ - getEnvVar: function(key, callback) { - const returnOrCallback = function(err, val) { - if (callback) { - callback(err, val); - return; - } - return val; - } - if (!envVarExcludes[key]) { - const item = getGlobalEnv(key); + getEnvVar: function(k) { + if (!envVarExcludes[k]) { + const item = getGlobalEnv(k); if (item) { - const val = redUtil.evaluateNodeProperty(item.value, item.type, null, null, callback); - if (callback) { - return; - } + const val = redUtil.evaluateNodeProperty(item.value, item.type, null, null, null); return val; } - return returnOrCallback(null, process.env[key]); + return process.env[k]; } - return returnOrCallback(undefined); + return undefined; }, diffNodes: diffNodes, mapEnvVarProperties: mapEnvVarProperties, diff --git a/packages/node_modules/@node-red/util/lib/util.js b/packages/node_modules/@node-red/util/lib/util.js index 9f711f4fa..ad05d45f1 100644 --- a/packages/node_modules/@node-red/util/lib/util.js +++ b/packages/node_modules/@node-red/util/lib/util.js @@ -18,7 +18,6 @@ /** * @mixin @node-red/util_util */ -/** @typedef {import('../../runtime/lib/flows/Flow.js').Flow} RuntimeLibFlowsFlow */ const clonedeep = require("lodash.clonedeep"); const jsonata = require("jsonata"); @@ -527,68 +526,37 @@ function setObjectProperty(msg,prop,value,createMissing) { return true; } -/** +/*! * Get value of environment variable. * @param {Node} node - accessing node * @param {String} name - name of variable - * @param {RuntimeLibFlowsFlow} flow_ - (optional) flow to check for setting - * @param {(err: Error, result: Any) => void} callback - (optional) called when the property is evaluated * @return {String} value of env var */ -function getSetting(node, name, flow_, callback) { - const returnOrCallback = (err, result) => { - if (callback) { - callback(err, result); - return - } - return result; - } +function getSetting(node, name, flow_) { if (node) { if (name === "NR_NODE_NAME") { - return returnOrCallback(null, node.name); + return node.name; } if (name === "NR_NODE_ID") { - return returnOrCallback(null, node.id); + return node.id; } if (name === "NR_NODE_PATH") { - return returnOrCallback(null, node._path); + return node._path; } } - - /** @type {RuntimeLibFlowsFlow} */ var flow = (flow_ ? flow_ : (node ? node._flow : null)); if (flow) { if (node && node.g) { const group = flow.getGroupNode(node.g); - if (callback) { - flow.getGroupEnvSetting(node, group, name, (e, [result, newName]) => { - if (e) { - callback(e); - return - } - if (result) { - callback(null, result.val); - return - } - name = newName; - flow.getSetting(name, callback); - }); - return - } else { - const [result, newName] = flow.getGroupEnvSetting(node, group, name); - if (result) { - return result.val; - } - name = newName; + const [result, newName] = flow.getGroupEnvSetting(node, group, name); + if (result) { + return result.val; } + name = newName; } - const fVal = flow.getSetting(name, callback) - if (callback) { - return - } - return fVal; + return flow.getSetting(name); } - return returnOrCallback(null, process.env[name]); + return process.env[name]; } @@ -600,34 +568,19 @@ function getSetting(node, name, flow_, callback) { * will return `Hello Joe!`. * @param {String} value - the string to parse * @param {Node} node - the node evaluating the property - * @param {(err: Error, result: Any) => void} callback - (optional) called when the property is evaluated * @return {String} The parsed string * @memberof @node-red/util_util */ -function evaluateEnvProperty(value, node, callback) { - const returnOrCallback = (err, result) => { - if (callback) { - callback(err, result); - return - } - return result; - } - /** @type {RuntimeLibFlowsFlow} */ +function evaluateEnvProperty(value, node) { var flow = (node && hasOwnProperty.call(node, "_flow")) ? node._flow : null; var result; if (/^\${[^}]+}$/.test(value)) { // ${ENV_VAR} var name = value.substring(2,value.length-1); - result = getSetting(node, name, flow, callback); - if (callback) { - return - } + result = getSetting(node, name, flow); } else if (!/\${\S+}/.test(value)) { // ENV_VAR - result = getSetting(node, value, flow, callback); - if (callback) { - return - } + result = getSetting(node, value, flow); } else { // FOO${ENV_VAR}BAR return value.replace(/\${([^}]+)}/g, function(match, name) { @@ -635,7 +588,8 @@ function evaluateEnvProperty(value, node, callback) { return (val === undefined)?"":val; }); } - return returnOrCallback(null, (result === undefined)?"":result); + return (result === undefined)?"":result; + } @@ -723,10 +677,7 @@ function evaluateNodeProperty(value, type, node, msg, callback) { return } } else if (type === 'env') { - result = evaluateEnvProperty(value, node, callback); - if (callback) { - return - } + result = evaluateEnvProperty(value, node); } if (callback) { callback(null,result); diff --git a/test/nodes/core/common/91-global-config_spec.js b/test/nodes/core/common/91-global-config_spec.js index 29dd905ae..8cc5658cf 100644 --- a/test/nodes/core/common/91-global-config_spec.js +++ b/test/nodes/core/common/91-global-config_spec.js @@ -44,30 +44,4 @@ describe('unknown Node', function() { }); }); - it('should evaluate a global environment variable that is a JSONata value', function (done) { - const flow = [{ - id: "n1", type: "global-config", name: "XYZ", - env: [ - { name: "now-var", type: "jsonata", value: "$millis()" } - ] - }, - { id: "n2", type: "inject", topic: "t1", payload: "now-var", payloadType: "env", wires: [["n3"]], z: "flow" }, - { id: "n3", type: "helper" } - ]; - helper.load([config, inject], flow, function () { - var n2 = helper.getNode("n2"); - var n3 = helper.getNode("n3"); - n3.on("input", (msg) => { - try { - const now = Date.now(); - msg.should.have.property("payload").and.be.approximately(now, 1000); - done(); - } catch (err) { - done(err); - } - }); - n2.receive({}); - }); - }); - }); diff --git a/test/unit/@node-red/runtime/lib/flows/Flow_spec.js b/test/unit/@node-red/runtime/lib/flows/Flow_spec.js index 9f99830b0..6d53747ef 100644 --- a/test/unit/@node-red/runtime/lib/flows/Flow_spec.js +++ b/test/unit/@node-red/runtime/lib/flows/Flow_spec.js @@ -686,7 +686,7 @@ describe('Flow', function() { },50); }); - it("passes a status event to the group scoped status node",function(done) { + it.only("passes a status event to the group scoped status node",function(done) { var config = flowUtils.parseConfig([ {id:"t1",type:"tab"}, {id: "g1", type: "group", g: "g3" }, @@ -1311,42 +1311,33 @@ describe('Flow', function() { }) process.env.V0 = "gv0"; process.env.V1 = "gv1"; - process.env.V3 = "gv3"; var config = flowUtils.parseConfig([ {id:"t1",type:"tab",env:[ - {"name": "V0", value: "t1v0", type: "str"}, - {"name": "V2", value: "t1v2", type: "str"} + {"name": "V0", value: "v0", type: "str"} ]}, {id:"g1",type:"group",z:"t1",env:[ - {"name": "V0", value: "g1v0", type: "str"}, - {"name": "V1", value: "g1v1", type: "str"} + {"name": "V0", value: "v1", type: "str"}, + {"name": "V1", value: "v2", type: "str"} ]}, {id:"g2",type:"group",z:"t1",g:"g1",env:[ - {"name": "V1", value: "g2v1", type: "str"} + {"name": "V1", value: "v3", type: "str"} ]}, - {id:"t1__V0",x:10,y:10,z:"t1",type:"test",foo:"${V0}",wires:[]}, // V0 will come from tab env V0 - {id:"t1g1V0",x:10,y:10,z:"t1",g:"g1",type:"test",foo:"${V0}",wires:[]}, // V0 will come from group 1 env V0 - {id:"t1g1V1",x:10,y:10,z:"t1",g:"g1",type:"test",foo:"${V1}",wires:[]}, // V1 will come from group 1 env V1 - {id:"t1g2V0",x:10,y:10,z:"t1",g:"g2",type:"test",foo:"${V0}",wires:[]}, // V0 will come from group 1 env V0 - {id:"t1g2V1",x:10,y:10,z:"t1",g:"g2",type:"test",foo:"${V1}",wires:[]}, // V1 will come from group 2 env V1 - {id:"t1g2V2",x:10,y:10,z:"t1",g:"g2",type:"test",foo:"${V2}",wires:[]}, // V2 will come from tab 1 env V2 - {id:"t1g2V3",x:10,y:10,z:"t1",g:"g2",type:"test",foo:"${V3}",wires:[]}, // V3 will come from process env V3 - - {id:"t1__V1",x:10,y:10,z:"t1",type:"test",foo:"${V1}",wires:[]}, + {id:"1",x:10,y:10,z:"t1",type:"test",foo:"$(V0)",wires:[]}, + {id:"2",x:10,y:10,z:"t1",g:"g1",type:"test",foo:"$(V0)",wires:[]}, + {id:"3",x:10,y:10,z:"t1",g:"g1",type:"test",foo:"$(V1)",wires:[]}, + {id:"4",x:10,y:10,z:"t1",g:"g2",type:"test",foo:"$(V1)",wires:[]}, + {id:"5",x:10,y:10,z:"t1",type:"test",foo:"$(V1)",wires:[]}, ]); var flow = Flow.create({getSetting:v=>process.env[v]},config,config.flows["t1"]); flow.start(); var activeNodes = flow.getActiveNodes(); - activeNodes.t1__V0.foo.should.equal("t1v0"); // node in tab 1, get tab 1 env V0 - activeNodes.t1__V1.foo.should.equal("gv1"); // node in tab 1, get V1, (tab 1 no V1) --> parent (global has V1) - activeNodes.t1g1V0.foo.should.equal("g1v0"); // node in group 1, get V0, (group 1 has V0) - activeNodes.t1g1V1.foo.should.equal("g1v1"); // node in group 1, get V1, (group 1 has V1) - activeNodes.t1g2V0.foo.should.equal("g1v0"); // node in group 2, get V0, (group 2 no V0) --> parent (group 1 has V0) - activeNodes.t1g2V1.foo.should.equal("g2v1"); // node in group 2, get V1, (group 2 has V1) - activeNodes.t1g2V2.foo.should.equal("t1v2"); // node in group 2, get V2, (group 2 no V2) --> parent (tab 1 has V2) - activeNodes.t1g2V3.foo.should.equal("gv3"); // node in group 2, get V3, (group 2 no V3) --> parent (tab 1 no V2) --> parent (global has V3) + activeNodes["1"].foo.should.equal("v0"); + activeNodes["2"].foo.should.equal("v1"); + activeNodes["3"].foo.should.equal("v2"); + activeNodes["4"].foo.should.equal("v3"); + activeNodes["5"].foo.should.equal("gv1"); flow.stop().then(function() { done(); @@ -1356,6 +1347,7 @@ describe('Flow', function() { console.log(e.stack); done(e); } + }); it("can access environment variable property using $parent", function (done) { try { @@ -1401,6 +1393,7 @@ describe('Flow', function() { console.log(e.stack); done(e); } + }); it("can define environment variable using JSONata", function (done) { @@ -1434,40 +1427,9 @@ describe('Flow', function() { console.log(e.stack); done(e); } + }); - it("can access global environment variables defined as JSONata values", function (done) { - try { - after(function() { - delete process.env.V0; - }) - var config = flowUtils.parseConfig([ - {id:"t1",type:"tab",env:[ - {"name": "V0", value: "1+2", type: "jsonata"} - ]}, - {id:"g1",type:"group",z:"t1",env:[ - {"name": "V1", value: "2+3", type: "jsonata"}, - ]}, - {id:"1",x:10,y:10,z:"t1",g:"g1",type:"test",foo:"$(V0)",wires:[]}, - {id:"2",x:10,y:10,z:"t1",g:"g1",type:"test",foo:"$(V1)",wires:[]}, - ]); - var flow = Flow.create({getSetting:v=>process.env[v]},config,config.flows["t1"]); - flow.start(); - - var activeNodes = flow.getActiveNodes(); - - activeNodes["1"].foo.should.equal(3); - activeNodes["2"].foo.should.equal(5); - - flow.stop().then(function() { - done(); - }); - } - catch (e) { - console.log(e.stack); - done(e); - } - }); }); }); diff --git a/test/unit/@node-red/runtime/lib/storage/localfilesystem/index_spec.js b/test/unit/@node-red/runtime/lib/storage/localfilesystem/index_spec.js index 00fab11cf..65826c9f4 100644 --- a/test/unit/@node-red/runtime/lib/storage/localfilesystem/index_spec.js +++ b/test/unit/@node-red/runtime/lib/storage/localfilesystem/index_spec.js @@ -489,7 +489,7 @@ describe('storage/localfilesystem', function() { var rootdir = path.win32.resolve(userDir+'/some'); // make it into a local UNC path flowFile = flowFile.replace('C:\\', '\\\\localhost\\c$\\'); - localfilesystem.init({userDir:userDir, flowFile:flowFile, getUserSettings: () => {{}}}, mockRuntime).then(function() { + localfilesystem.init({userDir:userDir, flowFile:flowFile}, mockRuntime).then(function() { fs.existsSync(flowFile).should.be.false(); localfilesystem.saveFlows(testFlow).then(function() { fs.existsSync(flowFile).should.be.true();