Skip to content

Commit

Permalink
#2302 Nodes with looped links crash the UI (#2303)
Browse files Browse the repository at this point in the history 
Signed-off-by: CTomlyn <[email protected]>
  • Loading branch information
@tomlyn
tomlyn authored Jan 21, 2025
1 parent 4f2a26f commit f722476
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 44 deletions.
10 changes: 5 additions & 5 deletions canvas_modules/common-canvas/__tests__/object-model/object-model-redux-test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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();

Expand Down Expand Up @@ -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));
Expand Down Expand Up @@ -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));
Expand Down
134 changes: 95 additions & 39 deletions canvas_modules/common-canvas/src/object-model/object-model.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down

0 comments on commit f722476

Please sign in to comment.