From f7224760c3d752c6f233f3744aefd2da0b320b39 Mon Sep 17 00:00:00 2001 From: CTomlyn Date: Tue, 21 Jan 2025 14:36:07 -0800 Subject: [PATCH] #2302 Nodes with looped links crash the UI (#2303) Signed-off-by: CTomlyn --- .../object-model/object-model-redux-test.js | 10 +- .../src/object-model/object-model.js | 134 +++++++++++++----- 2 files changed, 100 insertions(+), 44 deletions(-) diff --git a/canvas_modules/common-canvas/__tests__/object-model/object-model-redux-test.js b/canvas_modules/common-canvas/__tests__/object-model/object-model-redux-test.js index 4aff79f572..8588e8441d 100644 --- a/canvas_modules/common-canvas/__tests__/object-model/object-model-redux-test.js +++ b/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)); diff --git a/canvas_modules/common-canvas/src/object-model/object-model.js b/canvas_modules/common-canvas/src/object-model/object-model.js index 91f7160ee1..6018dbd5ae 100644 --- a/canvas_modules/common-canvas/src/object-model/object-model.js +++ b/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.