[dna-commits] DNA SVN: r1350 - trunk/dna-graph/src/main/java/org/jboss/dna/graph/session.

[dna-commits at lists.jboss.org]

Author: rhauch
Date: 2009-11-25 13:15:33 -0500 (Wed, 25 Nov 2009)
New Revision: 1350

Modified:
   trunk/dna-graph/src/main/java/org/jboss/dna/graph/session/GraphSession.java
Log:
DNA-559 GraphSession cloneImmediate fails to remove existing nodes from cache because of Set ordering variations

GraphSession.cloneImmediate(...) has a parameter that specifies whether any existing nodes should be removed (from the connector), and if this connector request succeeds and did remove those nodes, the method continues by attempting to purge these nodes from the cache. However, the request collects in a Set the Location for each of the removed nodes, so the order of these nodes cannot be guaranteed. And in fact when the cloneImmediate method walks them, the test was purging a parent node followed by a child node. Since purging the parent also purged all children (and other descendants), attempting to purge the child results in an exception.

The fix is relatively simple: check whether a node being purged is below a node that already was purged.  All unit tests now pass.

Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/session/GraphSession.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/session/GraphSession.java	2009-11-25 15:20:46 UTC (rev 1349)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/session/GraphSession.java	2009-11-25 18:15:33 UTC (rev 1350)
@@ -645,9 +645,16 @@
         Location locationOfCopy = request.getActualLocationAfter();
 
         // Remove from the session all of the nodes that were removed as part of this clone ...
+        Set<Path> removedAlready = new HashSet<Path>();
         for (Location removed : request.getRemovedNodes()) {
-            Node<Payload, PropertyPayload> removedNode = findNodeWith(removed.getPath(), false);
-            if (removedNode != null) removedNode.remove(false);
+            Path path = removed.getPath();
+            if (isBelow(path, removedAlready)) {
+                // This node is below a node we've already removed, so skip it ...
+                continue;
+            }
+            Node<Payload, PropertyPayload> removedNode = findNodeWith(path, false);
+            removedNode.remove(false);
+            removedAlready.add(path);
         }
 
         // Find the parent node in the session ...
@@ -658,6 +665,14 @@
         }
     }
 
+    private static final boolean isBelow( Path path,
+                                          Collection<Path> paths ) {
+        for (Path aPath : paths) {
+            if (aPath.isAncestorOf(path)) return true;
+        }
+        return false;
+    }
+
     /**
      * Refreshes (removes the cached state) for all cached nodes.
      * <p>