Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ensure edge exists before removal in Graph.removeEdge #8554

Merged
merged 6 commits into from Oct 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
25 changes: 20 additions & 5 deletions packages/core/graph/src/Graph.js
Expand Up @@ -142,7 +142,7 @@ export default class Graph<TNode, TEdgeType: number = 1> {
}

for (let {type, from} of this.adjacencyList.getInboundEdgesByType(nodeId)) {
this.removeEdge(
this._removeEdge(
from,
nodeId,
type,
Expand All @@ -153,7 +153,7 @@ export default class Graph<TNode, TEdgeType: number = 1> {
}

for (let {type, to} of this.adjacencyList.getOutboundEdgesByType(nodeId)) {
this.removeEdge(nodeId, to, type);
this._removeEdge(nodeId, to, type);
}

let wasRemoved = this.nodes.delete(nodeId);
Expand All @@ -166,16 +166,31 @@ export default class Graph<TNode, TEdgeType: number = 1> {
}

for (let to of this.getNodeIdsConnectedFrom(nodeId, type)) {
this.removeEdge(nodeId, to, type);
this._removeEdge(nodeId, to, type);
}
}

// Removes edge and node the edge is to if the node is orphaned
removeEdge(
from: NodeId,
to: NodeId,
type: TEdgeType | NullEdgeType = 1,
removeOrphans: boolean = true,
) {
if (!this.adjacencyList.hasEdge(from, to, type)) {
throw new Error(
`Edge from ${fromNodeId(from)} to ${fromNodeId(to)} not found!`,
);
}

this._removeEdge(from, to, type, removeOrphans);
}

// Removes edge and node the edge is to if the node is orphaned
_removeEdge(
from: NodeId,
to: NodeId,
type: TEdgeType | NullEdgeType = 1,
removeOrphans: boolean = true,
) {
if (!this.adjacencyList.hasEdge(from, to, type)) {
return;
Expand Down Expand Up @@ -249,7 +264,7 @@ export default class Graph<TNode, TEdgeType: number = 1> {
}

for (let child of childrenToRemove) {
this.removeEdge(fromNodeId, child, type);
this._removeEdge(fromNodeId, child, type);
}
}

Expand Down
10 changes: 10 additions & 0 deletions packages/core/graph/test/Graph.test.js
Expand Up @@ -89,6 +89,16 @@ describe('Graph', () => {
assert(!graph.isOrphanedNode(nodeC));
});

it("removeEdge should throw if the edge doesn't exist", () => {
let graph = new Graph();
let nodeA = graph.addNode('a');
let nodeB = graph.addNode('b');

assert.throws(() => {
graph.removeEdge(nodeA, nodeB);
}, /Edge from 0 to 1 not found!/);
});

it('removeEdge should prune the graph at that edge', () => {
// a
// / \
Expand Down