GraphStore.removeNode(Node) and GraphStore.removeAllNodes(Collection) throw IllegalArgumentException: Node should belong to a store when called with a NodeImpl whose storeId == NULL_ID (i.e. a node that was never added, or that has already been removed — typically a stale reference held by UI code). The exception is raised deep inside EdgeStore.checkValidNodeObject via edgeStore.edgeIterator(node, false), so the stack trace points to EdgeStore rather than the public API the caller invoked.
This is inconsistent with:
NodeStore.remove(Object) and EdgeStore.remove(Object), which silently return false for the same input.GraphStore.contains(Node), which also returns false silently.Collection.remove(Object) semantics in general.
Reported downstream in Gephi as GEPHI-5M2 (2 users, 5 events) and GEPHI-5P9 (1 user) on GraphElementsControllerImpl.deleteNode, where the trigger appears to be deleting a node twice (e.g. selection-driven delete actions racing with another removal).
Stack trace:
IllegalArgumentException: Node should belong to a store
at org.gephi.graph.impl.EdgeStore.checkValidNodeObject(EdgeStore.java:1236)
at org.gephi.graph.impl.EdgeStore.edgeIterator(EdgeStore.java:423)
at org.gephi.graph.impl.GraphStore.removeNode(GraphStore.java:280)
Proposed fix
In GraphStore.removeNode and GraphStore.removeAllNodes, after the null/type check, early-return false (or continue for the batch case) when node.storeId == NULL_ID. Aligns the public API with the underlying stores and Collection.remove semantics, and makes UI-side double-deletes a no-op instead of an exception.
Out of scope for this issue (worth tracking separately): the same stale-reference exception surfaces from many other Node-taking methods on GraphStore (getNeighbors, getEdges(Node), clearEdges(Node), getDegree, …) with equally misleading stack traces.
GraphStore.removeNode(Node)andGraphStore.removeAllNodes(Collection)throwIllegalArgumentException: Node should belong to a storewhen called with aNodeImplwhosestoreId == NULL_ID(i.e. a node that was never added, or that has already been removed — typically a stale reference held by UI code). The exception is raised deep insideEdgeStore.checkValidNodeObjectviaedgeStore.edgeIterator(node, false), so the stack trace points toEdgeStorerather than the public API the caller invoked.This is inconsistent with:
NodeStore.remove(Object)andEdgeStore.remove(Object), which silently returnfalsefor the same input.GraphStore.contains(Node), which also returnsfalsesilently.Collection.remove(Object)semantics in general.Reported downstream in Gephi as GEPHI-5M2 (2 users, 5 events) and GEPHI-5P9 (1 user) on
GraphElementsControllerImpl.deleteNode, where the trigger appears to be deleting a node twice (e.g. selection-driven delete actions racing with another removal).Stack trace:
Proposed fix
In
GraphStore.removeNodeandGraphStore.removeAllNodes, after the null/type check, early-returnfalse(orcontinuefor the batch case) whennode.storeId == NULL_ID. Aligns the public API with the underlying stores andCollection.removesemantics, and makes UI-side double-deletes a no-op instead of an exception.Out of scope for this issue (worth tracking separately): the same stale-reference exception surfaces from many other Node-taking methods on
GraphStore(getNeighbors,getEdges(Node),clearEdges(Node),getDegree, …) with equally misleading stack traces.