Link Call should not call into subflow...

- includes missing jsdoc
- improves speed (no searching, only lookups)
- code formatting consistency
- improve tests

packages/node_modules/@node-red/nodes/core/common/60-link.js

@@ -21,6 +21,7 @@
  * @property {string} name - Name of target Node
  * @property {number} flowId - ID of flow where the target node exists
  * @property {string} flowName - Name of flow where the target node exists
+ * @property {boolean} isSubFlow - True if the link-in node exists in a subflow instance
  */
 
 
@@ -28,7 +29,7 @@ module.exports = function(RED) {
     "use strict";
     const crypto = require("crypto");
     const targetCache = (function () {
-        const registry = { ids: {}, named: {}};
+        const registry = { id: {}, name: {} };
         function getIndex(/** @type {[LinkTarget]}*/ targets, id) {
             for (let index = 0; index < (targets || []).length; index++) {
                 const element = targets[index];
@@ -56,11 +57,16 @@ module.exports = function(RED) {
         return {
             /**
              * Get a list of targets registerd to this name
-             * @param {string} name 
+             * @param {string} name Name of the target
+             * @param {boolean} [excludeSubflows] set `true` to exclude 
              * @returns {[LinkTarget]} Targets registerd to this name.
              */
-            getTargets(name) {
+            getTargets(name, excludeSubflows) {
-                return registry.named[name] || [];
+                const targets = registry.name[name] || [];
+                if (excludeSubflows) {
+                    return targets.filter(e => e.isSubFlow != true);
+                }
+                return targets;
             },
             /**
              * Get a single target by registered name.
@@ -71,13 +77,13 @@ module.exports = function(RED) {
              * @returns {LinkTarget} target
              */
             getTarget(name, flowId) {
+                /** @type {[LinkTarget]}*/
                 let possibleTargets = this.getTargets(name);
-                /** @type {LinkTarget}*/ let target;
+                /** @type {LinkTarget}*/ 
-                if(possibleTargets.length) {
+                let target;
-                    if(flowId) {
+                if (possibleTargets.length && flowId) {
                     possibleTargets = possibleTargets.filter(e => e.flowId == flowId);
                 }
-                }
                 if (possibleTargets.length === 1) {
                     target = possibleTargets[0];
                 }
@@ -89,16 +95,16 @@ module.exports = function(RED) {
              * @returns {LinkTarget} target
              */
             getTargetById(nodeId) {
-                return registry.ids[nodeId];
+                return registry.id[nodeId];
             },
             register(/** @type {LinkInNode} */ node) {
                 const target = generateTarget(node);
                 const tByName = this.getTarget(target.name, target.flowId);
-                if(!tByName) {
+                if (!tByName || tByName.id !== target.id) {
-                    registry.named[target.name] = registry.named[target.name] || [];
+                    registry.name[target.name] = registry.name[target.name] || [];
-                    registry.named[target.name].push(target)
+                    registry.name[target.name].push(target)
                 }
-                registry.ids[target.id] = target;
+                registry.id[target.id] = target;
                 return target;
             },
             remove(node) {
@@ -111,13 +117,13 @@ module.exports = function(RED) {
                         targs.splice(idx, 1);
                     }
                     if (targs.length === 0) {
-                        delete registry.named[tn.name];
+                        delete registry.name[tn.name];
                     }
                 }
-                delete registry.ids[target.id];
+                delete registry.id[target.id];
             },
             clear() {
-                registry = { ids: {}, named: {}};
+                registry = { id: {}, name: {} };
             }
         }
     })();
@@ -187,17 +193,27 @@ module.exports = function(RED) {
         if (isNaN(timeout)) {
             timeout = 30000;
         }
-        function findNode(target) {
+        function getTargetNode(msg) {
-            let foundNode = RED.nodes.getNode(target); //1st see if the target is a direct node id
+            const dynamicMode = linkType === "dynamic";
-            if(!foundNode) {
+            const target = dynamicMode ? msg.target : staticTarget
-                //first look in this flow for the node
+
+            ////1st see if the target is a direct node id
+            let foundNode;
+            if (targetCache.getTargetById(target)) {
+                foundNode = RED.nodes.getNode(target)
+            }
+            if (target && !foundNode && dynamicMode) {
+                //next, look in **this flow only** for the node
                 let cachedTarget = targetCache.getTarget(target, node._flow.flow.id);
                 if (!cachedTarget) {
-                    //single target node not found in registry! get all possible targets
+                    //single target node not found in registry! 
-                    const possibleTargets = targetCache.getTargets(target);
+                    //get all possible targets from regular flows (exclude subflow instances)
+                    const possibleTargets = targetCache.getTargets(target, true);
                     if (possibleTargets.length === 1) {
+                        //only 1 link-in found with this name - good, lets use it
                         cachedTarget = possibleTargets[0];
                     } else if (possibleTargets.length > 1) {
+                        //more than 1 link-in has this name, raise an error
                         throw new Error(`Multiple link-in nodes named '${target}' found`);
                     }
                 }
@@ -208,12 +224,12 @@ module.exports = function(RED) {
             if (foundNode instanceof LinkInNode) {
                 return foundNode;
             }
-            throw new Error(`link-in node '${target}' not found`);
+            throw new Error(`target link-in node '${target || ""}' not found`);
         }
         this.on("input", function (msg, send, done) {
             try {
-                let targetNode = linkType == "dynamic" ? findNode(msg.target) : RED.nodes.getNode(staticTarget);
+                const targetNode = getTargetNode(msg);
-                if (targetNode && targetNode instanceof LinkInNode) {
+                if (targetNode instanceof LinkInNode) {
                     msg._linkSource = msg._linkSource || [];
                     const messageEvent = {
                         id: crypto.randomBytes(14).toString('hex'),

test/nodes/core/common/60-link_spec.js

@@ -148,12 +148,18 @@ describe('link Node', function() {
         })
 
         it('should call link-in node by name and get response', function (done) {
+            this.timeout(500);
             var payload = Date.now();
-            var flow = [{id:"link-in-1", type:"link in", name:"double payload", wires: [[ "func"]]},
+            var flow = [
-                        {id:"func", type:"helper", wires: [["link-out-1"]]},
+                { id: "tab-flow-1", type: "tab", label: "Flow 1" },
-                        {id:"link-out-1", type:"link out", mode: "return"},
+                { id: "tab-flow-2", type: "tab", label: "Flow 2" },
-                        {id:"link-call", type:"link call", linkType:"dynamic", links:[], wires:[["n4"]]},
+                { id: "link-in-1", z: "tab-flow-1", type: "link in", name: "double payload", wires: [["func"]] },
-                        {id:"n4", type:"helper"} ];
+                { id: "link-in-2", z: "tab-flow-2", type: "link in", name: "double payload", wires: [["func"]] },
+                { id: "func", z: "tab-flow-1", type: "helper", wires: [["link-out-1"]] },
+                { id: "link-out-1", z: "tab-flow-1", type: "link out", mode: "return" },
+                { id: "link-call", z: "tab-flow-1", type: "link call", linkType: "dynamic", links: [], wires: [["n4"]] },
+                { id: "n4", z: "tab-flow-1", type: "helper" }
+            ];
             helper.load(linkNode, flow, function () {
                 var func = helper.getNode("func");
                 func.on("input", function (msg, send, done) {
@@ -175,6 +181,40 @@ describe('link Node', function() {
                 n1.receive({ payload: payload, target: "double payload" });
             });
         })
+        // //TODO: This test is DISABLED - helper.load() calls callback but none of the nodes are available (issue loading a flow with a subflow)
+        // it('should call link-in node by name in subflow', function (done) {
+        //     this.timeout(9999500);
+        //     var payload = Date.now();
+        //     var flow = [
+        //         {"id":"sub-flow-template","type":"subflow","name":"Subflow","info":"","category":"","in":[{"wires":[{"id":"link-call-1"}]}],"out":[{"wires":[{"id":"link-call-1","port":0}]}]},
+        //         {"id":"link-call-1","type":"link call","z":"sub-flow-template","name":"","links":[],"linkType":"dynamic","timeout":"5","wires":[[]]},
+        //         {"id":"link-in-1","type":"link in","z":"sub-flow-template","name":"double payload","links":[],"wires":[["func"]]},
+        //         {"id":"func","type":"function","z":"sub-flow-template","name":"payload.a  x  payload.b","func":"msg.payload += msg.payload\nreturn msg;\n","outputs":1,"wires":[["link-out-1"]]},
+        //         {"id":"link-out-1","type":"link out","z":"sub-flow-template","name":"","mode":"return","links":[],"wires":[]},
+        //         {"id":"sub-flow-1","type":"subflow:sub-flow-template","z":"tab-flow-1","name":"","wires":[["n4"]]},
+        //         { id: "n4", z: "tab-flow-1", type: "helper" }
+        //     ];
+        //     helper.load(linkNode, flow, function () {
+        //         var sf = helper.getNode("sub-flow-1");
+        //         var func = helper.getNode("func");
+        //         var n4 = helper.getNode("n4");
+        //         func.on("input", function (msg, send, done) {
+        //             msg.payload += msg.payload;
+        //             send(msg);
+        //             done();
+        //         })
+        //         n4.on("input", function (msg) {
+        //             try {
+        //                 msg.should.have.property('payload');
+        //                 msg.payload.should.eql(payload + payload);
+        //                 done();
+        //             } catch (err) {
+        //                 done(err);
+        //             }
+        //         });
+        //         sf.receive({ payload: payload, target: "double payload" });
+        //     });
+        // })
         it('should timeout waiting for link return', function (done) {
             this.timeout(1000);
             const flow = [
@@ -209,7 +249,41 @@ describe('link Node', function() {
                 linkCallNode.receive({ payload: "hello", target: "double payload" });
             });
         })
-        it('should raise error due to multiple targets', function(done) {
+        it('should raise error due to multiple targets on same tab', function (done) {
+            this.timeout(55500);
+            const flow = [
+                { id: "tab-flow-1", type: "tab", label: "Flow 1" },
+                { id: "link-in-1", z: "tab-flow-1", type: "link in", name: "double payload", wires: [["func"]] },
+                { id: "link-in-2", z: "tab-flow-1", type: "link in", name: "double payload", wires: [["func"]] },
+                { id: "func", z: "tab-flow-1", type: "helper", wires: [["link-out-1"]] },
+                { id: "link-out-1", z: "tab-flow-1", type: "link out", mode: "return" },
+                { id: "link-call", z: "tab-flow-1", type: "link call", linkType: "dynamic", links: [], wires: [["n4"]] },
+                { id: "catch-all", z: "tab-flow-1", type: "catch", scope: ["link-call"], uncaught: true, wires: [["n4"]] },
+                { id: "n4", z: "tab-flow-1", type: "helper" }
+            ];
+            helper.load(linkNode, flow, function () {
+                const funcNode = helper.getNode("func");
+                const linkCall = helper.getNode("link-call");
+                const helperNode = helper.getNode("n4");
+                funcNode.on("input", function (msg, send, _done) {
+                    done(new Error("Function should not be called"))
+                })
+                helperNode.on("input", function (msg) {
+                    try {
+                        msg.should.have.property("target", "double payload");
+                        msg.should.have.property("error");
+                        msg.error.should.have.property("message");
+                        msg.error.message.should.match(/.*Multiple link-in nodes.*/)
+                        msg.error.should.have.property("source");
+                        done();
+                    } catch (err) {
+                        done(err);
+                    }
+                });
+                linkCall.receive({ payload: "hello", target: "double payload" });
+            });
+        })
+        it('should raise error due to multiple targets on different tabs', function (done) {
             this.timeout(500);
             const flow = [
                 { id: "tab-flow-1", type: "tab", label: "Flow 1" },