Fix nodes/edges are re-added to removedNodes / removedEdges on every subsequent diff because the cache entry is never nulled after detection

Author: mbastian

src/main/java/org/gephi/graph/impl/GraphObserverImpl.java

@@ -119,10 +119,11 @@ protected void refreshDiff() {
         if (nodeVersion < graphVersion.nodeVersion) {
             int maxStoreId = graphStore.nodeStore.maxStoreId();
 
-            for (Node n : nodeCache) {
-                NodeImpl nImpl = (NodeImpl) n;
+            for (int i = 0; i < nodeCache.length; i++) {
+                NodeImpl nImpl = nodeCache[i];
                 if (nImpl != null && !graph.contains(nImpl)) {
                     graphDiff.removedNodes.add(nImpl);
+                    nodeCache[i] = null;
                 }
             }
             if (maxStoreId > nodeCache.length || maxStoreId < nodeCache.length) {
@@ -145,10 +146,11 @@ protected void refreshDiff() {
         if (edgeVersion < graphVersion.edgeVersion) {
             int maxStoreId = graphStore.edgeStore.maxStoreId();
 
-            for (Edge e : edgeCache) {
-                EdgeImpl eImpl = (EdgeImpl) e;
+            for (int i = 0; i < edgeCache.length; i++) {
+                EdgeImpl eImpl = edgeCache[i];
                 if (eImpl != null && !graph.contains(eImpl)) {
                     graphDiff.removedEdges.add(eImpl);
+                    edgeCache[i] = null;
                 }
             }
             if (maxStoreId > edgeCache.length || maxStoreId < edgeCache.length) {

src/test/java/org/gephi/graph/impl/GraphObserverTest.java

@@ -389,6 +389,57 @@ public void testDiffRemoveAllNodes() {
         Assert.assertSame(diff.getAddedNodes(), NodeIterable.EMPTY);
     }
 
+    @Test
+    public void testDiffRemovedNodeNotReportedTwice() {
+        GraphStore store = GraphGenerator.generateSmallGraphStore();
+        GraphObserverImpl graphObserver = store.createGraphObserver(store, true);
+        graphObserver.hasGraphChanged();
+
+        Node removed = store.getNodes().toArray()[0];
+        store.removeNode(removed);
+
+        // First diff: removal is reported
+        graphObserver.hasGraphChanged();
+        GraphDiff diff1 = graphObserver.getDiff();
+        Assert.assertEquals(diff1.getRemovedNodes().toArray().length, 1);
+
+        // Make an unrelated change so a second diff is triggered
+        store.addNode(store.factory.newNode("extra"));
+
+        // Second diff: the already-reported removal must NOT appear again
+        graphObserver.hasGraphChanged();
+        GraphDiff diff2 = graphObserver.getDiff();
+        for (Node n : diff2.getRemovedNodes()) {
+            Assert.assertNotSame(n, removed);
+        }
+    }
+
+    @Test
+    public void testDiffRemovedEdgeNotReportedTwice() {
+        GraphStore store = GraphGenerator.generateSmallGraphStore();
+        GraphObserverImpl graphObserver = store.createGraphObserver(store, true);
+        graphObserver.hasGraphChanged();
+
+        Edge removed = store.getEdges().toArray()[0];
+        store.removeEdge(removed);
+
+        // First diff: removal is reported
+        graphObserver.hasGraphChanged();
+        GraphDiff diff1 = graphObserver.getDiff();
+        Assert.assertEquals(diff1.getRemovedEdges().toArray().length, 1);
+
+        // Make an unrelated change so a second diff is triggered
+        Node[] ns = store.getNodes().toArray();
+        store.addEdge(store.factory.newEdge("extra", ns[0], ns[1], 0, 1.0, true));
+
+        // Second diff: the already-reported removal must NOT appear again
+        graphObserver.hasGraphChanged();
+        GraphDiff diff2 = graphObserver.getDiff();
+        for (Edge e : diff2.getRemovedEdges()) {
+            Assert.assertNotSame(e, removed);
+        }
+    }
+
     @Test
     public void testDiffReplaceNode() {
         GraphStore store = GraphGenerator.generateSmallGraphStore();