#2302 Nodes with looped links crash the UI

canvas_modules/common-canvas/__tests__/object-model/object-model-redux-test.js

@@ -1229,7 +1229,7 @@ describe("ObjectModel handle model OK", () => {
 
 		objectModel.selectSubGraph("node4", "123");
 
-		const expectedSelections = ["node2", "node4", "node3"];
+		const expectedSelections = ["node2", "node3", "node4"];
 		const actualSelections = objectModel.getSelectedObjectIds();
 
 		// console.info("Expected Canvas = " + JSON.stringify(expectedSelections, null, 4));
@@ -1421,7 +1421,7 @@ describe("ObjectModel handle model OK", () => {
 
 		objectModel.selectSubGraph("node13", "123");
 
-		const expectedSelections = ["node1", "node13", "node2", "node3", "node4", "node11", "node12",
+		const expectedSelections = ["node1", "node2", "node3", "node4", "node11", "node13", "node12",
 			"node5", "node6", "node7"];
 		const actualSelections = objectModel.getSelectedObjectIds();
 
@@ -1486,7 +1486,7 @@ describe("ObjectModel handle model OK", () => {
 
 		objectModel.selectSubGraph("node12", "123");
 
-		const expectedSelections = ["node1", "node12", "node2", "node3", "node4",
+		const expectedSelections = ["node1", "node2", "node3", "node4", "node12",
 			"node5", "node6", "node7"];
 		const actualSelections = objectModel.getSelectedObjectIds();
 
@@ -1551,7 +1551,7 @@ describe("ObjectModel handle model OK", () => {
 
 		objectModel.selectSubGraph("node11", "123");
 
-		const expectedSelections = ["node8", "node11", "node4", "node12"];
+		const expectedSelections = ["node8", "node4", "node11", "node12"];
 		const actualSelections = objectModel.getSelectedObjectIds();
 
 		// console.info("Expected Selections = " + JSON.stringify(expectedSelections));
@@ -1615,7 +1615,7 @@ describe("ObjectModel handle model OK", () => {
 
 		objectModel.selectSubGraph("node13", "123");
 
-		const expectedSelections = ["comment1", "node13", "node7", "node4", "node11", "node12"];
+		const expectedSelections = ["comment1", "node7", "node4", "node11", "node13", "node12"];
 		const actualSelections = objectModel.getSelectedObjectIds();
 
 		// console.info("Expected Canvas = " + JSON.stringify(expectedSelections, null, 4));

canvas_modules/common-canvas/src/object-model/object-model.js

@@ -1562,60 +1562,116 @@ export default class ObjectModel {
 		this.setSelections([], apiPipeline.pipelineId);
 	}
 
-	findNodesInSubGraph(startNodeId, endNodeId, selection, pipelineId) {
-		const pipeline = this.getAPIPipeline(pipelineId);
-		let retval = false;
+	// Selects a set of nodes which represet all connected nodes from the
+	// current set of selected nodes to the end node passed in. If no
+	// connecting nodes are found, the set of selected nodes remains the same.
+	selectSubGraph(endNodeId, pipelineId) {
+		const selectedObjectIds = this.getSubGraphNodes(endNodeId, pipelineId);
+		this.setSelections(selectedObjectIds, pipelineId);
+	}
 
-		selection.push(startNodeId);
-		if (startNodeId === endNodeId) {
-			retval = true;
-		} else {
-			for (const link of pipeline.getLinks()) {
-				if (link.srcNodeId === startNodeId &&
-					link.srcNodeId !== link.trgNodeId) { // Ignore self-referencing links
-					const newRetval = this.findNodesInSubGraph(link.trgNodeId, endNodeId, selection, pipelineId);
-					if (newRetval !== true) {
-						selection.pop();
+	// Returns an array of node IDs that represent all connected nodes from the
+	// current set of selected nodes to the end node passed in.
+	getSubGraphNodes(endNodeId, pipelineId) {
+		const selectedObjectIds = this.getSelectedObjectIds();
+
+		if (pipelineId === this.getSelectedPipelineId()) {
+			const pipeline = this.getAPIPipeline(pipelineId);
+			const links = pipeline.getLinks();
+			const allPaths = [];
+
+			// Loop through all the currently selected nodes which will be
+			// our start nodes.
+			for (const startNodeId of selectedObjectIds) {
+				const paths = this.getGraphPaths(startNodeId, endNodeId, links);
+				allPaths.push(...paths);
+			}
+
+			// Add to the set of currently selected object IDs any nodes
+			// found that connect from them to the end node.
+			for (const path of allPaths) {
+				for (const nodeId of path) {
+					if (!selectedObjectIds.includes(nodeId)) {
+						selectedObjectIds.push(nodeId);
 					}
-					// This handles the case where there are multiple outward paths.
-					// Some of the outward paths could be true and some false. This
-					// will make sure that the node in the selection list of one of the
-					// paths contains the end nodeId.
-					retval = retval || newRetval;
 				}
 			}
 		}
 
-		return retval;
+		// Make sure the end node is included in the list of selected nodes.
+		if (!selectedObjectIds.includes(endNodeId)) {
+			selectedObjectIds.push(endNodeId);
+		}
+
+		return selectedObjectIds;
 	}
 
-	selectSubGraph(endNodeId, pipelineId) {
-		const selection = [];
-		let selectedObjectIds = [endNodeId];
+	// Returns an array of paths where each path is an array of nodes from
+	// the start node to the end node.
+	getGraphPaths(startNodeId, endNodeId, links) {
+		const visited = new Set();
+		const path = [];
+		const paths = [];
 
-		if (pipelineId === this.getSelectedPipelineId()) {
-			const currentSelectedObjects = this.getSelectedObjectIds();
+		this.getGraphPathForNode(startNodeId, endNodeId, path, visited, paths, links);
 
-			// Get all the nodes in the path from a currently selected object to the end node
-			let foundPath = false;
-			for (const startNodeId of currentSelectedObjects) {
-				foundPath = foundPath || this.findNodesInSubGraph(startNodeId, endNodeId, selection, pipelineId);
-			}
-			if (!foundPath) {
-				// If no subgraph found which is also the case if current selection was empty, just select endNode
-				selection.push(endNodeId);
+		return paths;
+	}
+
+	// Updates the paths array with any new path found where a path is an array
+	// of node IDs that connect the node passed in to the end node. To do this it uses
+	// the path and visited arrays as working arrays.
+	getGraphPathForNode(nodeId, endNodeId, path, visited, paths, links) {
+		if (nodeId === endNodeId) {
+			paths.push([...path, endNodeId]);
+			return;
+		}
+
+		if (visited.has(nodeId)) {
+			// If we've visited this node before, and the node ID is in one of the
+			// currently saved paths, we make the path to this node an additional
+			// path even though it doesn't end at the end node. All nodes in paths
+			//  will get consolidated by our caller so that is not a problem.
+			if (this.isInSavedPath(nodeId, paths)) {
+				paths.push([...path, endNodeId]);
 			}
+			return;
+		}
 
-			// Do not put multiple copies of a nodeId in selected nodeId list.
-			selectedObjectIds = this.getSelectedObjectIds().slice();
-			for (const selected of selection) {
-				if (!this.isSelected(selected, pipelineId)) {
-					selectedObjectIds.push(selected);
-				}
+		visited.add(nodeId);
+		path.push(nodeId);
+
+		const neighbors = this.getNeighbourNodeIDs(nodeId, links);
+
+		for (const neighbor of neighbors) {
+			this.getGraphPathForNode(neighbor, endNodeId, path, visited, paths, links);
+		}
+
+		path.pop();
+	}
+
+	// Returns true if the node ID passed in is in one of the
+	// paths stored in the paths array.
+	isInSavedPath(nodeId, paths) {
+		for (const path of paths) {
+			if (path.includes(nodeId)) {
+				return true;
 			}
 		}
+		return false;
+	}
 
-		this.setSelections(selectedObjectIds, pipelineId);
+	// Returns an array of neighbor nodes for the node identified
+	// by the ID passed in.
+	getNeighbourNodeIDs(nodeId, links) {
+		const neighbors = [];
+
+		links.forEach((l) => {
+			if (l.srcNodeId === nodeId && l.trgNodeId && l.type !== ASSOCIATION_LINK) {
+				neighbors.push(l.trgNodeId);
+			}
+		});
+		return neighbors;
 	}
 
 	// Return true is nodeIds are contiguous.