[dna-commits] DNA SVN: r903 - in trunk/dna-graph/src: test/java/org/jboss/dna/graph/request and 1 other directory.

[dna-commits at lists.jboss.org]

Author: rhauch
Date: 2009-05-14 10:48:09 -0400 (Thu, 14 May 2009)
New Revision: 903

Modified:
   trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/ReadBranchRequest.java
   trunk/dna-graph/src/test/java/org/jboss/dna/graph/request/AbstractRequestTest.java
   trunk/dna-graph/src/test/java/org/jboss/dna/graph/request/ReadBranchRequestTest.java
Log:
DNA-407 ReadBranchRequest.iterator() returns too many nodes

Verified the behavior with additional unit tests, and then corrected the iterator to not include children that are not actually part of the branch.

Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/ReadBranchRequest.java
===================================================================
--- trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/ReadBranchRequest.java	2009-05-13 16:36:56 UTC (rev 902)
+++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/ReadBranchRequest.java	2009-05-14 14:48:09 UTC (rev 903)
@@ -337,7 +337,11 @@
                 Location next = queue.poll();
                 if (next == null) throw new NoSuchElementException();
                 List<Location> children = getChildren(next);
-                if (children != null && children.size() > 0) queue.addAll(0, children);
+                if (children != null && children.size() > 0) {
+                    // We should only add the children if they are nodes in the branch, so check the first one...
+                    Location firstChild = children.get(0);
+                    if (includes(firstChild)) queue.addAll(0, children);
+                }
                 return next;
             }
 

Modified: trunk/dna-graph/src/test/java/org/jboss/dna/graph/request/AbstractRequestTest.java
===================================================================
--- trunk/dna-graph/src/test/java/org/jboss/dna/graph/request/AbstractRequestTest.java	2009-05-13 16:36:56 UTC (rev 902)
+++ trunk/dna-graph/src/test/java/org/jboss/dna/graph/request/AbstractRequestTest.java	2009-05-14 14:48:09 UTC (rev 903)
@@ -93,6 +93,10 @@
         return context.getValueFactories().getNameFactory().create(name);
     }
 
+    protected Location location( String path ) {
+        return Location.create(createPath(path));
+    }
+
     protected abstract Request createRequest();
 
     @Test

Modified: trunk/dna-graph/src/test/java/org/jboss/dna/graph/request/ReadBranchRequestTest.java
===================================================================
--- trunk/dna-graph/src/test/java/org/jboss/dna/graph/request/ReadBranchRequestTest.java	2009-05-13 16:36:56 UTC (rev 902)
+++ trunk/dna-graph/src/test/java/org/jboss/dna/graph/request/ReadBranchRequestTest.java	2009-05-14 14:48:09 UTC (rev 903)
@@ -27,6 +27,8 @@
 import static org.hamcrest.core.IsNull.nullValue;
 import static org.hamcrest.core.IsSame.sameInstance;
 import static org.junit.Assert.assertThat;
+import java.util.Iterator;
+import org.jboss.dna.graph.Location;
 import org.junit.Before;
 import org.junit.Test;
 
@@ -104,4 +106,67 @@
         ReadBranchRequest request2 = new ReadBranchRequest(validPathLocation1, workspace1, 2);
         assertThat(request.equals(request2), is(false));
     }
+
+    @Test
+    public void shouldIterateOverNodesInBranchOfDepthOne() {
+        request = new ReadBranchRequest(location("/a"), workspace1, 1);
+        request.setActualLocationOfNode(request.at());
+        request.setChildren(location("/a"), location("/a/b"), location("/a/c"));
+        Iterator<Location> actual = request.iterator();
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a")));
+        assertThat(actual.hasNext(), is(false));
+    }
+
+    @Test
+    public void shouldIterateOverNodesInBranchOfDepthTwo() {
+        request = new ReadBranchRequest(location("/a"), workspace1, 2);
+        request.setActualLocationOfNode(request.at());
+        request.setChildren(location("/a"), location("/a/b"), location("/a/c"), location("/a/d"));
+        request.setChildren(location("/a/b"), location("/a/b/j"), location("/a/b/k"));
+        request.setChildren(location("/a/c"), location("/a/c/j"), location("/a/c/k"));
+        request.setChildren(location("/a/d"));
+        Iterator<Location> actual = request.iterator();
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a")));
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a/b")));
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a/c")));
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a/d")));
+        assertThat(actual.hasNext(), is(false));
+    }
+
+    @Test
+    public void shouldIterateOverNodesInBranchOfDepthThree() {
+        request = new ReadBranchRequest(location("/a"), workspace1, 3);
+        request.setActualLocationOfNode(request.at());
+        request.setChildren(location("/a"), location("/a/b"), location("/a/c"), location("/a/d"));
+        request.setChildren(location("/a/b"), location("/a/b/j"), location("/a/b/k"));
+        request.setChildren(location("/a/c"), location("/a/c/j"), location("/a/c/k"));
+        request.setChildren(location("/a/c/j"), location("/a/c/j/j1"), location("/a/c/j/j2"));
+        request.setChildren(location("/a/c/k"), location("/a/c/k/k1"), location("/a/c/k/k2"));
+        request.setChildren(location("/a/b/j"), location("/a/b/j/m"), location("/a/b/j/n"));
+        request.setChildren(location("/a/b/k"));
+        request.setChildren(location("/a/d"));
+        Iterator<Location> actual = request.iterator();
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a")));
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a/b")));
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a/b/j")));
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a/b/k")));
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a/c")));
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a/c/j")));
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a/c/k")));
+        assertThat(actual.hasNext(), is(true));
+        assertThat(actual.next(), is(location("/a/d")));
+        assertThat(actual.hasNext(), is(false));
+    }
 }