#58 Harden component return validation for junctions

Author: ollixx

components/component-return.html

@@ -8,36 +8,54 @@
 		const MSG_NOT_CONNECTED = "notConnected";
 		const MSG_TOO_MANY_START_NODES = "tooManyStartNodes";
 
-		const findStartNodes = function (nodeId, sources = {}, visited = [], found = []) {
-			if (visited.includes(nodeId)) {
+		const getEndpointId = function (endpoint) {
+			if (!endpoint) {
+				return null;
+			}
+			if (typeof endpoint === "string") {
+				return endpoint;
+			}
+			return endpoint.id || null;
+		};
+
+		const findStartNodes = function (nodeId, visited = new Set(), found = new Set()) {
+			if (!nodeId || visited.has(nodeId)) {
 				return found;
 			}
-			visited.push(nodeId);
-			let node = RED.nodes.node(nodeId);
+
+			visited.add(nodeId);
+			const node = RED.nodes.node(nodeId);
 			if (!node) {
 				return found;
 			}
-			if (node.type == "component_in") {
-				found.push(nodeId);
+
+			if (node.type === "component_in") {
+				found.add(nodeId);
 			}
+
 			RED.nodes.eachLink((link) => {
-				if (link.target.id == nodeId) {
-					sources[link.source.id] = {};
-					findStartNodes(link.source.id, sources[link.source.id], visited, found);
+				if (getEndpointId(link && link.target) !== nodeId) {
+					return;
 				}
-			});
-			if (node.type == "link in" && node.links) {
-				for (let lout of node.links) {
-					sources[lout] = {};
-					findStartNodes(lout, sources[lout], visited, found);
+
+				const sourceId = getEndpointId(link && link.source);
+				if (sourceId) {
+					findStartNodes(sourceId, visited, found);
 				}
+			});
+
+			if (node.type === "link in" && Array.isArray(node.links)) {
+				node.links.forEach((linkOutId) => {
+					findStartNodes(linkOutId, visited, found);
+				});
 			}
+
 			return found;
 		};
 
 		const getValidationResult = function (node) {
-			let startNodes = findStartNodes(node.id);
-			if (startNodes.length == 0) {
+			const startNodes = Array.from(findStartNodes(node && node.id));
+			if (startNodes.length === 0) {
 				return {
 					codes: [MSG_NOT_CONNECTED],
 					message: RED._("components.message.componentNotConnected")

components/lib/editor-graph.js

@@ -0,0 +1,71 @@
+function getEndpointId(endpoint) {
+    if (!endpoint) {
+        return null;
+    }
+
+    if (typeof endpoint === "string") {
+        return endpoint;
+    }
+
+    return endpoint.id || null;
+}
+
+function findStartNodes(RED, nodeId, visited = new Set(), found = new Set()) {
+    if (!nodeId || visited.has(nodeId)) {
+        return found;
+    }
+
+    visited.add(nodeId);
+
+    const node = RED.nodes.node(nodeId);
+    if (!node) {
+        return found;
+    }
+
+    if (node.type === "component_in") {
+        found.add(nodeId);
+    }
+
+    RED.nodes.eachLink((link) => {
+        if (getEndpointId(link && link.target) !== nodeId) {
+            return;
+        }
+
+        const sourceId = getEndpointId(link && link.source);
+        if (sourceId) {
+            findStartNodes(RED, sourceId, visited, found);
+        }
+    });
+
+    if (node.type === "link in" && Array.isArray(node.links)) {
+        node.links.forEach((linkOutId) => {
+            findStartNodes(RED, linkOutId, visited, found);
+        });
+    }
+
+    return found;
+}
+
+function getComponentReturnValidationResult(RED, node) {
+    const startNodes = Array.from(findStartNodes(RED, node && node.id));
+    if (startNodes.length === 0) {
+        return {
+            codes: ["notConnected"],
+            message: RED._("components.message.componentNotConnected")
+        };
+    }
+
+    if (startNodes.length > 1) {
+        return {
+            codes: ["tooManyStartNodes"],
+            message: RED._("components.message.returnWithoutStart", { inNodeLength: startNodes.length })
+        };
+    }
+
+    return { codes: [], message: "" };
+}
+
+module.exports = {
+    findStartNodes,
+    getComponentReturnValidationResult
+};

components/test/editor_graph_spec.js

@@ -0,0 +1,81 @@
+var should = require("should");
+var editorGraph = require("../lib/editor-graph");
+
+function createEditorRed(nodes, links) {
+    return {
+        _: function (key, args) {
+            if (key === "components.message.returnWithoutStart") {
+                return key + ":" + args.inNodeLength;
+            }
+            return key;
+        },
+        nodes: {
+            node: function (id) {
+                return nodes[id] || null;
+            },
+            eachLink: function (callback) {
+                links.forEach(callback);
+            }
+        }
+    };
+}
+
+describe("editor graph helpers", function () {
+    it("should validate component_out through junction links", function () {
+        var red = createEditorRed(
+            {
+                in01: { id: "in01", type: "component_in" },
+                junction01: { id: "junction01", type: "junction" },
+                ret01: { id: "ret01", type: "component_out" }
+            },
+            [
+                { source: { id: "in01" }, target: { id: "junction01" } },
+                { source: { id: "junction01" }, target: { id: "ret01" } }
+            ]
+        );
+
+        editorGraph.getComponentReturnValidationResult(red, { id: "ret01" }).should.eql({
+            codes: [],
+            message: ""
+        });
+    });
+
+    it("should ignore malformed links while finding start nodes", function () {
+        var red = createEditorRed(
+            {
+                in01: { id: "in01", type: "component_in" },
+                junction01: { id: "junction01", type: "junction" },
+                ret01: { id: "ret01", type: "component_out" }
+            },
+            [
+                { source: { id: "in01" }, target: { id: "junction01" } },
+                { source: "junction01", target: "ret01" },
+                { source: undefined, target: { id: "ret01" } },
+                { source: { id: "missing" }, target: null }
+            ]
+        );
+
+        Array.from(editorGraph.findStartNodes(red, "ret01")).should.eql(["in01"]);
+    });
+
+    it("should report multiple component starts as invalid", function () {
+        var red = createEditorRed(
+            {
+                in01: { id: "in01", type: "component_in" },
+                in02: { id: "in02", type: "component_in" },
+                junction01: { id: "junction01", type: "junction" },
+                ret01: { id: "ret01", type: "component_out" }
+            },
+            [
+                { source: { id: "in01" }, target: { id: "junction01" } },
+                { source: { id: "in02" }, target: { id: "junction01" } },
+                { source: { id: "junction01" }, target: { id: "ret01" } }
+            ]
+        );
+
+        editorGraph.getComponentReturnValidationResult(red, { id: "ret01" }).should.eql({
+            codes: ["tooManyStartNodes"],
+            message: "components.message.returnWithoutStart:2"
+        });
+    });
+});