Index: dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/ForkRequestProcessor.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/ForkRequestProcessor.java (revision 938) +++ dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/ForkRequestProcessor.java (working copy) @@ -1129,6 +1129,7 @@ if (projectedFromNode == null) return; ProjectedNode projectedIntoNode = project(request.into(), request.inWorkspace(), request, true); if (projectedIntoNode == null) return; + ProjectedNode projectedBeforeNode = request.before() != null ? project(request.before(), request.inWorkspace(), request, true) : null; // Limitation: only able to project the move if the 'from' and 'into' are in the same source & projection ... while (projectedFromNode != null) { @@ -1160,11 +1161,12 @@ ProxyNode fromProxy = projectedFromNode.asProxy(); ProxyNode intoProxy = projectedIntoNode.asProxy(); + ProxyNode beforeProxy = request.before() != null ? projectedBeforeNode.asProxy() : null; assert fromProxy.projection().getSourceName().equals(intoProxy.projection().getSourceName()); boolean sameLocation = fromProxy.isSameLocationAsOriginal() && intoProxy.isSameLocationAsOriginal(); // Create the pushed-down request ... - MoveBranchRequest pushDown = new MoveBranchRequest(fromProxy.location(), intoProxy.location(), intoProxy.workspaceName(), + MoveBranchRequest pushDown = new MoveBranchRequest(fromProxy.location(), intoProxy.location(), beforeProxy.location(), intoProxy.workspaceName(), request.desiredName(), request.conflictBehavior()); // Create the federated request ... FederatedRequest federatedRequest = new FederatedRequest(request); Index: dna-graph/src/main/java/org/jboss/dna/graph/connector/inmemory/InMemoryRepository.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/connector/inmemory/InMemoryRepository.java (revision 938) +++ dna-graph/src/main/java/org/jboss/dna/graph/connector/inmemory/InMemoryRepository.java (working copy) @@ -351,16 +351,19 @@ * @param desiredNewName the new name for the node, if it is to be changed; may be null * @param newWorkspace the workspace containing the new parent node * @param newParent the new parent; may not be the {@link Workspace#getRoot() root} + * @param beforeNode the node before which this new node should be placed */ public void moveNode( ExecutionContext context, InMemoryNode node, Name desiredNewName, Workspace newWorkspace, - InMemoryNode newParent ) { + InMemoryNode newParent, + InMemoryNode beforeNode) { assert context != null; assert newParent != null; assert node != null; - assert newWorkspace.getRoot().equals(newParent) != true; +// Why was this restriction here? -- BRC +// assert newWorkspace.getRoot().equals(newParent) != true; assert this.getRoot().equals(node) != true; InMemoryNode oldParent = node.getParent(); Name oldName = node.getName().getName(); @@ -377,9 +380,16 @@ newName = desiredNewName; node.setName(context.getValueFactories().getPathFactory().createSegment(desiredNewName, 1)); } - newParent.getChildren().add(node); + + if (beforeNode == null) { + newParent.getChildren().add(node); + } + else { + int index = newParent.getChildren().indexOf(beforeNode); + newParent.getChildren().add(index, node); + } correctSameNameSiblingIndexes(context, newParent, newName); - + // If the node was moved to a new workspace... if (!this.equals(newWorkspace)) { // We need to remove the node from this workspace's map of nodes ... Index: dna-graph/src/main/java/org/jboss/dna/graph/connector/inmemory/InMemoryRequestProcessor.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/connector/inmemory/InMemoryRequestProcessor.java (revision 938) +++ dna-graph/src/main/java/org/jboss/dna/graph/connector/inmemory/InMemoryRequestProcessor.java (working copy) @@ -222,7 +222,8 @@ // Look up the new parent, which must exist ... Path newParentPath = request.into().getPath(); InMemoryNode newParent = workspace.getNode(newParentPath); - workspace.moveNode(getExecutionContext(), node, request.desiredName(), workspace, newParent); + InMemoryNode beforeNode = request.before() != null ? workspace.getNode(request.before().getPath()) : null; + workspace.moveNode(getExecutionContext(), node, request.desiredName(), workspace, newParent, beforeNode); assert node.getParent() == newParent; Path newPath = getExecutionContext().getValueFactories().getPathFactory().create(newParentPath, node.getName()); Location oldLocation = getActualLocation(request.from().getPath(), node); Index: dna-graph/src/main/java/org/jboss/dna/graph/request/BatchRequestBuilder.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/request/BatchRequestBuilder.java (revision 938) +++ dna-graph/src/main/java/org/jboss/dna/graph/request/BatchRequestBuilder.java (working copy) @@ -627,7 +627,7 @@ Location into, String workspaceName, Name newNameForNode ) { - return add(new MoveBranchRequest(from, into, workspaceName, newNameForNode, MoveBranchRequest.DEFAULT_CONFLICT_BEHAVIOR)); + return add(new MoveBranchRequest(from, into, null, workspaceName, newNameForNode, MoveBranchRequest.DEFAULT_CONFLICT_BEHAVIOR)); } /** @@ -635,7 +635,26 @@ * * @param from the location of the top node in the existing branch that is to be moved * @param into the location of the existing node into which the branch should be moved + * @param before the location of the node before which the branch should be moved; may be null * @param workspaceName the name of the workspace + * @param newNameForNode the new name for the node being moved, or null if the name of the original should be used + * @return this builder for method chaining; never null + * @throws IllegalArgumentException if any of the parameters are null + */ + public BatchRequestBuilder moveBranch( Location from, + Location into, + Location before, + String workspaceName, + Name newNameForNode ) { + return add(new MoveBranchRequest(from, into, before, workspaceName, newNameForNode, MoveBranchRequest.DEFAULT_CONFLICT_BEHAVIOR)); + } + + /** + * Create a request to move a branch from one location into another. + * + * @param from the location of the top node in the existing branch that is to be moved + * @param into the location of the existing node into which the branch should be moved + * @param workspaceName the name of the workspace * @param conflictBehavior the expected behavior if an equivalently-named child already exists at the into * location * @return this builder for method chaining; never null Index: dna-graph/src/main/java/org/jboss/dna/graph/request/MoveBranchRequest.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/request/MoveBranchRequest.java (revision 938) +++ dna-graph/src/main/java/org/jboss/dna/graph/request/MoveBranchRequest.java (working copy) @@ -44,6 +44,7 @@ private final Location from; private final Location into; + private final Location before; private final String workspaceName; private final Name desiredNameForNode; private final NodeConflictBehavior conflictBehavior; @@ -61,7 +62,7 @@ public MoveBranchRequest( Location from, Location into, String workspaceName ) { - this(from, into, workspaceName, null, DEFAULT_CONFLICT_BEHAVIOR); + this(from, into, null, workspaceName, null, DEFAULT_CONFLICT_BEHAVIOR); } /** @@ -77,7 +78,7 @@ Location into, String workspaceName, Name newNameForMovedNode ) { - this(from, into, workspaceName, newNameForMovedNode, DEFAULT_CONFLICT_BEHAVIOR); + this(from, into, null, workspaceName, newNameForMovedNode, DEFAULT_CONFLICT_BEHAVIOR); } /** @@ -94,7 +95,7 @@ Location into, String workspaceName, NodeConflictBehavior conflictBehavior ) { - this(from, into, workspaceName, null, conflictBehavior); + this(from, into, null, workspaceName, null, conflictBehavior); } /** @@ -102,6 +103,8 @@ * * @param from the location of the top node in the existing branch that is to be moved * @param into the location of the existing node into which the branch should be moved + * @param before the location of the child of the {@code into} node that the branch should be placed before; null indicates + * that the branch should be the last child of its new parent * @param workspaceName the name of the workspace * @param newNameForMovedNode the new name for the node being moved, or null if the name of the original should be used * @param conflictBehavior the expected behavior if an equivalently-named child already exists at the into @@ -110,6 +113,7 @@ */ public MoveBranchRequest( Location from, Location into, + Location before, String workspaceName, Name newNameForMovedNode, NodeConflictBehavior conflictBehavior ) { @@ -119,6 +123,7 @@ CheckArg.isNotNull(conflictBehavior, "conflictBehavior"); this.from = from; this.into = into; + this.before = before; this.workspaceName = workspaceName; this.desiredNameForNode = newNameForMovedNode; this.conflictBehavior = conflictBehavior; @@ -143,6 +148,15 @@ } /** + * Get the location defining the node before which the branch is to be placed + * + * @return the to location; null indicates that the branch should be the last child node of its new parent + */ + public Location before() { + return before; + } + + /** * Get the name of the workspace in which the branch exists. * * @return the name of the workspace containing the branch; never null Index: dna-graph/src/main/java/org/jboss/dna/graph/request/RequestBuilder.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/request/RequestBuilder.java (revision 938) +++ dna-graph/src/main/java/org/jboss/dna/graph/request/RequestBuilder.java (working copy) @@ -474,9 +474,28 @@ Location into, String workspaceName, Name newNameForNode ) { - return process(new MoveBranchRequest(from, into, workspaceName, newNameForNode, MoveBranchRequest.DEFAULT_CONFLICT_BEHAVIOR)); + return process(new MoveBranchRequest(from, into, null, workspaceName, newNameForNode, MoveBranchRequest.DEFAULT_CONFLICT_BEHAVIOR)); } + /** + * Create a request to move a branch from one location into another before the given child node of the new location. + * + * @param from the location of the top node in the existing branch that is to be moved + * @param into the location of the existing node into which the branch should be moved + * @param before the location of the node before which the branch should be moved; may be null + * @param workspaceName the name of the workspace + * @param newNameForNode the new name for the node being moved, or null if the name of the original should be used + * @return the request; never null + * @throws IllegalArgumentException if any of the parameters are null + */ + public MoveBranchRequest moveBranch( Location from, + Location into, + Location before, + String workspaceName, + Name newNameForNode ) { + return process(new MoveBranchRequest(from, into, before, workspaceName, newNameForNode, MoveBranchRequest.DEFAULT_CONFLICT_BEHAVIOR)); + } + /** * Create a request to move a branch from one location into another. * * @param from the location of the top node in the existing branch that is to be moved @@ -494,7 +513,7 @@ Name newNameForNode, NodeConflictBehavior conflictBehavior ) { if (conflictBehavior == null) conflictBehavior = MoveBranchRequest.DEFAULT_CONFLICT_BEHAVIOR; - return process(new MoveBranchRequest(from, into, workspaceName, newNameForNode, conflictBehavior)); + return process(new MoveBranchRequest(from, into, null, workspaceName, newNameForNode, conflictBehavior)); } /** Index: dna-graph/src/test/java/org/jboss/dna/graph/connector/inmemory/InMemoryRepositoryWorkspaceTest.java =================================================================== --- dna-graph/src/test/java/org/jboss/dna/graph/connector/inmemory/InMemoryRepositoryWorkspaceTest.java (revision 938) +++ dna-graph/src/test/java/org/jboss/dna/graph/connector/inmemory/InMemoryRepositoryWorkspaceTest.java (working copy) @@ -241,7 +241,7 @@ assertThat(workspace.getNode(pathFactory.create("/d/e")), is(sameInstance(node_e))); assertThat(workspace.getNode(pathFactory.create("/d/b")), is(sameInstance(node_b2))); - workspace.moveNode(context, node_b, null, workspace, node_d); + workspace.moveNode(context, node_b, null, workspace, node_d, null); assertThat(workspace.getNode(pathFactory.create("/")), is(sameInstance(workspace.getRoot()))); assertThat(workspace.getNode(pathFactory.create("/a")), is(sameInstance(node_a))); @@ -251,7 +251,7 @@ assertThat(workspace.getNode(pathFactory.create("/d/b[2]")), is(sameInstance(node_b))); assertThat(workspace.getNode(pathFactory.create("/d/b[2]/c")), is(sameInstance(node_c))); - workspace.moveNode(context, node_b, null, workspace, node_e); + workspace.moveNode(context, node_b, null, workspace, node_e, null); assertThat(workspace.getNode(pathFactory.create("/")), is(sameInstance(workspace.getRoot()))); assertThat(workspace.getNode(pathFactory.create("/a")), is(sameInstance(node_a))); @@ -263,6 +263,55 @@ } @Test + public void shouldMoveNodeBeforeAnother() { + InMemoryNode root = workspace.getRoot(); + InMemoryNode node_a = workspace.createNode(context, root, nameFactory.create("a"), null); + InMemoryNode node_b = workspace.createNode(context, node_a, nameFactory.create("b"), null); + InMemoryNode node_c = workspace.createNode(context, node_b, nameFactory.create("c"), null); + InMemoryNode node_d = workspace.createNode(context, root, nameFactory.create("d"), null); + InMemoryNode node_e = workspace.createNode(context, node_d, nameFactory.create("e"), null); + InMemoryNode node_b2 = workspace.createNode(context, node_d, nameFactory.create("b"), null); + Name propName = nameFactory.create("prop"); + node_b.setProperty(propertyFactory.create(propName, "node_b")); + node_b2.setProperty(propertyFactory.create(propName, "node_b2")); + + assertThat(workspace.getNodesByUuid().size(), is(7)); + assertThat(workspace.getNode(pathFactory.create("/")), is(sameInstance(workspace.getRoot()))); + assertThat(workspace.getNode(pathFactory.create("/a")), is(sameInstance(node_a))); + assertThat(workspace.getNode(pathFactory.create("/a/b")), is(sameInstance(node_b))); + assertThat(workspace.getNode(pathFactory.create("/a/b/c")), is(sameInstance(node_c))); + assertThat(workspace.getNode(pathFactory.create("/d")), is(sameInstance(node_d))); + assertThat(workspace.getNode(pathFactory.create("/d/e")), is(sameInstance(node_e))); + assertThat(workspace.getNode(pathFactory.create("/d/b")), is(sameInstance(node_b2))); + assertThat(workspace.getNode(pathFactory.create("/a/b")).getProperty(propName).getFirstValue().toString(), is("node_b")); + assertThat(workspace.getNode(pathFactory.create("/d/b")).getProperty(propName).getFirstValue().toString(), is("node_b2")); + + // Move before a node with the same name + workspace.moveNode(context, node_b, null, workspace, node_d, node_b2); + + assertThat(workspace.getNode(pathFactory.create("/")), is(sameInstance(workspace.getRoot()))); + assertThat(workspace.getNode(pathFactory.create("/a")), is(sameInstance(node_a))); + assertThat(workspace.getNode(pathFactory.create("/d")), is(sameInstance(node_d))); + assertThat(workspace.getNode(pathFactory.create("/d/e")), is(sameInstance(node_e))); + assertThat(workspace.getNode(pathFactory.create("/d/b[2]")), is(sameInstance(node_b2))); + assertThat(workspace.getNode(pathFactory.create("/d/b[1]")), is(sameInstance(node_b))); + assertThat(workspace.getNode(pathFactory.create("/d/b[1]/c")), is(sameInstance(node_c))); + assertThat(workspace.getNode(pathFactory.create("/d/b[1]")).getProperty(propName).getFirstValue().toString(), is("node_b")); + assertThat(workspace.getNode(pathFactory.create("/d/b[2]")).getProperty(propName).getFirstValue().toString(), is("node_b2")); + + // Move after the last node + workspace.moveNode(context, node_b, null, workspace, root, null); + + assertThat(workspace.getNode(pathFactory.create("/")), is(sameInstance(workspace.getRoot()))); + assertThat(workspace.getNode(pathFactory.create("/a")), is(sameInstance(node_a))); + assertThat(workspace.getNode(pathFactory.create("/d")), is(sameInstance(node_d))); + assertThat(workspace.getNode(pathFactory.create("/d/e")), is(sameInstance(node_e))); + assertThat(workspace.getNode(pathFactory.create("/b")), is(sameInstance(node_b))); + assertThat(workspace.getNode(pathFactory.create("/b/c")), is(sameInstance(node_c))); + assertThat(workspace.getNode(pathFactory.create("/d/b")), is(sameInstance(node_b2))); + } + + @Test public void shouldMoveNodesFromOneWorkspaceToAnother() { // Populate the workspace with some content ... InMemoryNode root = workspace.getRoot(); @@ -304,7 +353,7 @@ assertThat(new_workspace.getNode(pathFactory.create("/d/b")), is(sameInstance(new_node_b2))); // Move 'workspace::/a/b' into 'newWorkspace::/d' - workspace.moveNode(context, node_b, null, new_workspace, new_node_d); + workspace.moveNode(context, node_b, null, new_workspace, new_node_d, null); assertThat(workspace.getNodesByUuid().size(), is(5)); assertThat(workspace.getNode(pathFactory.create("/")), is(sameInstance(workspace.getRoot()))); Index: dna-graph/src/test/java/org/jboss/dna/graph/request/MoveBranchRequestTest.java =================================================================== --- dna-graph/src/test/java/org/jboss/dna/graph/request/MoveBranchRequestTest.java (revision 938) +++ dna-graph/src/test/java/org/jboss/dna/graph/request/MoveBranchRequestTest.java (working copy) @@ -27,6 +27,9 @@ import static org.hamcrest.core.IsNull.nullValue; import static org.hamcrest.core.IsSame.sameInstance; import static org.junit.Assert.assertThat; +import org.jboss.dna.graph.NodeConflictBehavior; +import org.jboss.dna.graph.property.Name; +import org.jboss.dna.graph.property.basic.BasicName; import org.junit.Before; import org.junit.Test; @@ -74,6 +77,18 @@ } @Test + public void shouldCreateValidRequestWithValidFromLocationAndValidToLocationAndValidBeforeLocation() { + Name newName = new BasicName("", "newName"); + request = new MoveBranchRequest(validPathLocation1, validPathLocation2, validPathLocation, workspace1, newName, NodeConflictBehavior.DO_NOT_REPLACE); + assertThat(request.from(), is(sameInstance(validPathLocation1))); + assertThat(request.into(), is(sameInstance(validPathLocation2))); + assertThat(request.before(), is(sameInstance(validPathLocation))); + assertThat(request.inWorkspace(), is(sameInstance(workspace1))); + assertThat(request.hasError(), is(false)); + assertThat(request.getError(), is(nullValue())); + } + + @Test public void shouldConsiderEqualTwoRequestsWithSameLocations() { request = new MoveBranchRequest(validPathLocation1, validPathLocation2, workspace2); MoveBranchRequest request2 = new MoveBranchRequest(validPathLocation1, validPathLocation2, workspace2);