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)); + } }