[dna-commits] DNA SVN: r1180 - in trunk: dna-graph/src/main/java/org/jboss/dna/graph/connector/map and 9 other directories.

Author: rhauch Date: 2009-08-28 11:26:27 -0400 (Fri, 28 Aug 2009) New Revision: 1180 Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinMirrorRequestProcessor.java trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinRequestProcessor.java trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/AbstractMapWorkspace.java trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapRepository.java trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapRequestProcessor.java trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapWorkspace.java trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/CloneBranchRequest.java trunk/dna-graph/src/main/java/org/jboss/dna/graph/session/GraphSession.java trunk/dna-graph/src/test/java/org/jboss/dna/graph/connector/test/WritableConnectorTest.java trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrI18n.java trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/JcrI18n.properties trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java trunk/extensions/dna-connector-federation/src/main/java/org/jboss/dna/connector/federation/FederatingRequestProcessor.java trunk/extensions/dna-connector-filesystem/src/main/java/org/jboss/dna/connector/filesystem/FileSystemRequestProcessor.java trunk/extensions/dna-connector-store-jpa/src/main/java/org/jboss/dna/connector/store/jpa/model/basic/BasicRequestProcessor.java Log: DNA-508 Workspace move/clone referenceable nodes is failing One of the problems was that the workspace move method implementation (down in GraphSession) was not changing the children (of the new and old parent) to reflect the newly removed/added children. The correction was simply a call to an existing method (similar to what was being done in copy). The second problem was the fact that clone was not removing from the session state (again, down in GraphSession) the nodes that were being removed during the clone operation, and therefore the session state had nodes that no longer existed in the persistent store. Unfortunately, the set of nodes that were indeed removed as part of the clone operation was not available after the fact, so correcting this behavior required adding the Set<Location> of these deleted nodes into the CloneBranchRequest. This in turn mandated that all writable connector implementations be changed to set this information as part of the processing of these requests. Once this was done, the GraphSession could be changed to use this information to remove these nodes from its cache. Incidentally, I discovered another possible error condition regarding clone. Consider a session node A, which has the same UUID as node B in another workspace. If that session attempts to clone node B (from the other workspace) and the clone method's removeExisting parameter is true, then node A needs to be removed in order for the clone to succeed. So far this is the normal situation. However, if node A has transient changes, then it cannot be removed from the session. Therefore, I changed the JcrWorkspace.clone(...) method to check for this case and throw a RepositoryException. (The spec did not delineate this error condition, so I'm assuming that this is the best type of exception to throw.) Luckily, the method was already checking that the would-be-deleted nodes were not mandatory, so also checking whether the node has local changes was very easy. Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinMirrorRequestProcessor.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinMirrorRequestProcessor.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinMirrorRequestProcessor.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -174,7 +174,8 @@ */ @Override public void process( ReadNextBlockOfChildrenRequest request ) { - ReadNextBlockOfChildrenRequest source = (ReadNextBlockOfChildrenRequest)federatedRequest.getFirstProjectedRequest().getRequest(); + ReadNextBlockOfChildrenRequest source = (ReadNextBlockOfChildrenRequest)federatedRequest.getFirstProjectedRequest() + .getRequest(); if (checkErrorOrCancel(request, source)) return; request.setActualLocationOfStartingAfterNode(source.getActualLocationOfStartingAfterNode()); for (Location childInSource : source.getChildren()) { @@ -324,6 +325,7 @@ CloneBranchRequest source = (CloneBranchRequest)federatedRequest.getFirstProjectedRequest().getRequest(); if (checkErrorOrCancel(request, source)) return; request.setActualLocations(source.getActualLocationBefore(), source.getActualLocationAfter()); + request.setRemovedNodes(source.getRemovedNodes()); } /** Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinRequestProcessor.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinRequestProcessor.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/federation/JoinRequestProcessor.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -24,11 +24,14 @@ package org.jboss.dna.graph.connector.federation; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.NoSuchElementException; +import java.util.Set; import java.util.concurrent.BlockingQueue; import net.jcip.annotations.NotThreadSafe; import org.jboss.dna.graph.ExecutionContext; @@ -768,7 +771,7 @@ * Project the supplied location in a source into its federated location. The projection is used to find the location under * the supplied ancestor. Any errors are recorded on the original request. * - * @param ancestorInFederation the ancestor in the federated repository; may not be nul + * @param ancestorInFederation the ancestor in the federated repository; may not be null * @param projection the projection that should be used; may not be null * @param actualSourceLocation the actual location in the source that is to be projected back into the federated repository; * may not be null @@ -795,6 +798,31 @@ } /** + * Project the supplied location in a source into its federated location. The projection is used to find the location under + * the supplied ancestor. Any errors are recorded on the original request. + * + * @param projection the projection that should be used; may not be null + * @param actualSourceLocation the actual location in the source that is to be projected back into the federated repository; + * may not be null + * @param originalRequest the original request, if there are errors; may not be null + * @return the location in the federated repository + */ + protected Location projectToFederated( Projection projection, + Location actualSourceLocation, + Request originalRequest ) { + Path actualPathInSource = actualSourceLocation.getPath(); + // Project the actual location ... + for (Path path : projection.getPathsInRepository(actualPathInSource, pathFactory)) { + return actualSourceLocation.with(path); + } + // Record that there was an error projecting the results ... + String whereInSource = actualSourceLocation.getString(getExecutionContext().getNamespaceRegistry()); + String msg = GraphI18n.unableToProjectSourceInformationIntoWorkspace.text(whereInSource, getSourceName(), projection); + originalRequest.setError(new InvalidRequestException(msg)); + return null; + } + + /** * {@inheritDoc} * * @see org.jboss.dna.graph.request.processor.RequestProcessor#process(org.jboss.dna.graph.request.CreateNodeRequest) @@ -981,6 +1009,13 @@ locationBefore = projectToFederated(request.from(), projected.getProjection(), locationBefore, request); locationAfter = projectToFederated(request.into(), projected.getSecondProjection(), locationAfter, request); request.setActualLocations(locationBefore, locationAfter); + if (source.removeExisting()) { + Set<Location> removed = new HashSet<Location>(); + for (Location location : request.getRemovedNodes()) { + removed.add(projectToFederated(projected.getSecondProjection(), location, request)); + } + request.setRemovedNodes(Collections.unmodifiableSet(removed)); + } } /** Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/AbstractMapWorkspace.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/AbstractMapWorkspace.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/AbstractMapWorkspace.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -34,6 +34,7 @@ import java.util.Set; import java.util.UUID; import org.jboss.dna.graph.ExecutionContext; +import org.jboss.dna.graph.Location; import org.jboss.dna.graph.connector.UuidAlreadyExistsException; import org.jboss.dna.graph.property.Name; import org.jboss.dna.graph.property.NamespaceRegistry; @@ -357,13 +358,13 @@ assert this.getRoot().equals(node) != true; MapNode oldParent = node.getParent(); Name oldName = node.getName().getName(); - + if (this.equals(newAbstractMapWorkspace) && node.getParent().getUuid().equals(newParent.getUuid()) && node.equals(beforeNode)) { // Trivial move of a node to its parent before itself return; } - + if (oldParent != null) { boolean removed = oldParent.getChildren().remove(node); assert removed == true; @@ -526,7 +527,7 @@ /** * {@inheritDoc} * - * @see MapWorkspace#cloneNode(ExecutionContext, MapNode, MapWorkspace, MapNode, Name, Segment, boolean) + * @see MapWorkspace#cloneNode(ExecutionContext, MapNode, MapWorkspace, MapNode, Name, Segment, boolean, Set) */ public MapNode cloneNode( ExecutionContext context, MapNode original, @@ -534,7 +535,8 @@ MapNode newParent, Name desiredName, Segment desiredSegment, - boolean removeExisting ) throws UuidAlreadyExistsException { + boolean removeExisting, + Set<Location> removedExistingNodes ) throws UuidAlreadyExistsException { assert context != null; assert original != null; assert newWorkspace != null; @@ -543,17 +545,23 @@ Set<UUID> uuidsInFromBranch = getUuidsUnderNode(original); MapNode existing; + PathFactory pathFactory = context.getValueFactories().getPathFactory(); + // TODO: Need to handle removing/throwing root node if (removeExisting) { - + // Remove all of the nodes that have a UUID from under the original node, but DO NOT yet remove + // a node that has the UUID of the original node (as this will be handled later) ... for (UUID uuid : uuidsInFromBranch) { if (null != (existing = newWorkspace.getNode(uuid))) { newWorkspace.removeNode(context, existing); + if (removedExistingNodes != null) { + Path path = pathFor(pathFactory, existing); + removedExistingNodes.add(Location.create(path, uuid)); + } } } } else { - PathFactory pathFactory = context.getValueFactories().getPathFactory(); - uuidsInFromBranch.add(original.getUuid()); + uuidsInFromBranch.add(original.getUuid()); // uuidsInFromBranch does not include the UUID of the original for (UUID uuid : uuidsInFromBranch) { if (null != (existing = newWorkspace.getNode(uuid))) { NamespaceRegistry namespaces = context.getNamespaceRegistry(); @@ -589,17 +597,21 @@ return newRoot; } + // Now deal with an existing node that has the same UUID as the original node ... existing = newWorkspace.getNode(original.getUuid()); - if (existing != null) { newWorkspace.removeNode(context, existing); + if (removedExistingNodes != null) { + Path path = pathFor(pathFactory, existing); + removedExistingNodes.add(Location.create(path, original.getUuid())); + } } return copyNode(context, original, newWorkspace, newParent, desiredName, true, (Map<UUID, UUID>)null); } /** - * Returns all of the UUIDs in the branch rooted at {@code node}. The UUID of {@code node} will not be included in the set of - * returned UUIDs. + * Returns all of the UUIDs in the branch rooted at {@code node}. The UUID of {@code node} <i>will</i> be included in the set + * of returned UUIDs. * * @param node the root of the branch * @return all of the UUIDs in the branch rooted at {@code node} @@ -607,7 +619,6 @@ public Set<UUID> getUuidsUnderNode( MapNode node ) { Set<UUID> uuids = new HashSet<UUID>(); uuidsUnderNode(node, uuids); - return uuids; } Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapRepository.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapRepository.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapRepository.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -248,7 +248,7 @@ // Loop over each child and call this method to copy the immediate children (and below). // Note that this makes the copy have the same UUID as the original. for (MapNode originalNode : origRoot.getChildren()) { - original.cloneNode(context, originalNode, workspace, root, originalNode.getName().getName(), null, true); + original.cloneNode(context, originalNode, workspace, root, originalNode.getName().getName(), null, true, null); } } Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapRequestProcessor.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapRequestProcessor.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapRequestProcessor.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -23,6 +23,7 @@ */ package org.jboss.dna.graph.connector.map; +import java.util.Collections; import java.util.HashSet; import java.util.LinkedList; import java.util.List; @@ -132,17 +133,20 @@ // Look up the new parent, which must exist ... Path newParentPath = request.into().getPath(); MapNode newParent = newWorkspace.getNode(newParentPath); + Set<Location> removedExistingNodes = new HashSet<Location>(); MapNode newNode = workspace.cloneNode(getExecutionContext(), node, newWorkspace, newParent, request.desiredName(), request.desiredSegment(), - request.removeExisting()); + request.removeExisting(), + removedExistingNodes); Path newPath = getExecutionContext().getValueFactories().getPathFactory().create(newParentPath, newNode.getName()); Location oldLocation = getActualLocation(request.from(), node); Location newLocation = Location.create(newPath, newNode.getUuid()); request.setActualLocations(oldLocation, newLocation); + request.setRemovedNodes(Collections.unmodifiableSet(removedExistingNodes)); recordChange(request); } @@ -278,6 +282,7 @@ MapNode beforeNode = request.before() != null ? getTargetNode(workspace, request, request.before()) : null; MapNode node = getTargetNode(workspace, request, request.from()); if (node == null) return; + if (request.hasError()) return; // if beforeNode could not be found // Look up the new parent, which must exist ... Path newParentPath; @@ -298,6 +303,12 @@ } MapNode newParent = workspace.getNode(newParentPath); + if (newParent == null) { + Path lowestExisting = workspace.getLowestExistingPath(newParentPath); + request.setError(new PathNotFoundException(request.into(), lowestExisting, + GraphI18n.inMemoryNodeDoesNotExist.text(newParentPath))); + return; + } workspace.moveNode(getExecutionContext(), node, request.desiredName(), workspace, newParent, beforeNode); assert node.getParent() == newParent; Path newPath = getExecutionContext().getValueFactories().getPathFactory().create(newParentPath, node.getName()); Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapWorkspace.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapWorkspace.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/connector/map/MapWorkspace.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -23,8 +23,10 @@ */ package org.jboss.dna.graph.connector.map; +import java.util.Set; import java.util.UUID; import org.jboss.dna.graph.ExecutionContext; +import org.jboss.dna.graph.Location; import org.jboss.dna.graph.connector.UuidAlreadyExistsException; import org.jboss.dna.graph.property.Name; import org.jboss.dna.graph.property.Path; @@ -162,6 +164,8 @@ * @param removeExisting true if existing nodes in the new workspace with the same UUIDs as nodes in the branch rooted at * {@code original} should be removed; if false, a UuidAlreadyExistsException will be thrown if a UUID conflict is * detected + * @param removedExistingNodes the set into which should be placed all of the existing nodes that were removed as a result of + * this clone operation, or null if these nodes need not be collected * @return the new node, which is the top of the new subgraph * @throws UuidAlreadyExistsException if {@code removeExisting} is true and and a UUID in the source tree already exists in * the new workspace @@ -172,7 +176,8 @@ MapNode newParent, Name desiredName, Path.Segment desiredSegment, - boolean removeExisting ) throws UuidAlreadyExistsException; + boolean removeExisting, + Set<Location> removedExistingNodes ) throws UuidAlreadyExistsException; /** * Find the lowest existing node along the path. Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/CloneBranchRequest.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/CloneBranchRequest.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/CloneBranchRequest.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -1,5 +1,7 @@ package org.jboss.dna.graph.request; +import java.util.Collections; +import java.util.Set; import org.jboss.dna.common.util.CheckArg; import org.jboss.dna.common.util.HashCode; import org.jboss.dna.graph.GraphI18n; @@ -33,6 +35,7 @@ private final Name desiredName; private final Path.Segment desiredSegment; private final boolean removeExisting; + private Set<Location> removedExistingNodes; private Location actualFromLocation; private Location actualIntoLocation; @@ -225,6 +228,24 @@ } /** + * Set the locations of the nodes that were removed by this operation, if {@link #removeExisting()} is true. + * + * @param existingNodesThatWereRemoved the (immutable) set of existing node locations; may be null + */ + public void setRemovedNodes( Set<Location> existingNodesThatWereRemoved ) { + this.removedExistingNodes = existingNodesThatWereRemoved; + } + + /** + * Get the set of nodes that were removed because of this clone operation. + * + * @return the immutable set of locations of the nodes that were removed; never null but possibly empty + */ + public Set<Location> getRemovedNodes() { + return removedExistingNodes != null ? removedExistingNodes : Collections.<Location>emptySet(); + } + + /** * {@inheritDoc} * * @see org.jboss.dna.graph.request.ChangeRequest#changes(java.lang.String, org.jboss.dna.graph.property.Path) 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-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/session/GraphSession.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -62,6 +62,7 @@ import org.jboss.dna.graph.request.CloneBranchRequest; import org.jboss.dna.graph.request.CopyBranchRequest; import org.jboss.dna.graph.request.InvalidWorkspaceException; +import org.jboss.dna.graph.request.MoveBranchRequest; import org.jboss.dna.graph.request.Request; import org.jboss.dna.graph.session.GraphSession.Authorizer.Action; import com.google.common.collect.ListMultimap; @@ -486,7 +487,17 @@ authorizer.checkPermissions(newParentPath, Action.ADD_NODE); authorizer.checkPermissions(nodeToMove.getParent(), Action.REMOVE); - store.move(nodeToMove).as(newName).into(newParentPath); + // Perform the move operation, but use a batch so that we can read the latest list of children ... + Results results = store.batch().move(nodeToMove).as(newName).into(newParentPath).execute(); + MoveBranchRequest moveRequest = (MoveBranchRequest)results.getRequests().get(0); + Location locationAfter = moveRequest.getActualLocationAfter(); + + // Find the parent node in the session ... + Node<Payload, PropertyPayload> parent = this.findNodeWith(locationAfter.getPath().getParent(), false); + if (parent != null && parent.isLoaded()) { + // Update the children to make them match the latest snapshot from the store ... + parent.synchronizeWithNewlyPersistedNode(locationAfter); + } } /** @@ -633,6 +644,12 @@ CloneBranchRequest request = (CloneBranchRequest)results.getRequests().get(0); Location locationOfCopy = request.getActualLocationAfter(); + // Remove from the session all of the nodes that were removed as part of this clone ... + for (Location removed : request.getRemovedNodes()) { + Node<Payload, PropertyPayload> removedNode = findNodeWith(removed.getPath(), false); + if (removedNode != null) removedNode.remove(false); + } + // Find the parent node in the session ... Node<Payload, PropertyPayload> parent = this.findNodeWith(locationOfCopy.getPath().getParent(), false); if (parent != null && parent.isLoaded()) { @@ -2097,13 +2114,26 @@ * is added to a different parent. However, the locations of same-name-siblings under the parent <i>are</i> updated. */ protected void remove() { + remove(true); + } + + /** + * Remove this node from it's parent. Note that locations are <i>not</i> updated, since they will be updated if this node + * is added to a different parent. However, the locations of same-name-siblings under the parent <i>are</i> updated. + * + * @param markParentAsChanged true if the parent should be marked as being changed (i.e., when changes are initiated from + * within this session), or false otherwise (i.e., when changes are made to reflect the persistent state) + */ + protected void remove( boolean markParentAsChanged ) { assert !isStale(); assert this.parent != null; assert this.parent.isLoaded(); assert this.parent.childrenByName != null; assert this.parent.childrenByName != cache.NO_CHILDREN; - this.parent.markAsChanged(); - this.markAsChanged(); + if (markParentAsChanged) { + this.parent.markAsChanged(); + this.markAsChanged(); + } Name name = getName(); List<Node<Payload, PropertyPayload>> childrenWithSameName = this.parent.childrenByName.get(name); this.parent = null; Modified: trunk/dna-graph/src/test/java/org/jboss/dna/graph/connector/test/WritableConnectorTest.java =================================================================== --- trunk/dna-graph/src/test/java/org/jboss/dna/graph/connector/test/WritableConnectorTest.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-graph/src/test/java/org/jboss/dna/graph/connector/test/WritableConnectorTest.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -1001,7 +1001,7 @@ graph.create("/segmentTestUuids/node1", propFactory.create(name("identifier"), "backup copy")).and(); // Copy again to test the behavior now that the UUIDs are already in the default workspace - // This should remove /newUuids/node1/shouldBeRemoved + // This should remove /segmentTestUuids/node1[1] graph.clone("/node1") .fromWorkspace(workspaceName) .as(segment("node1[1]")) @@ -1011,7 +1011,9 @@ /* * Focus on testing node structure, since shouldCopyNodeWithChildren tests that properties get copied */ - // /newUuids/node1 should have been removed when the new node was added with the same UUID + // /segmentTestUuids/node1[1] should have been removed when the new node was added with the same UUID + // Now "/segmentTestUuids/node1[1]" (which was "/segmentTestUuids/node1[2]") should have same UUID as the + // node with "identifier=backup copy" property assertThat(graph.getNodeAt("/segmentTestUuids").getChildren(), hasChildren(segment("node1"), segment("node1[2]"))); assertThat(graph.getNodeAt("/segmentTestUuids/node1[1]").getProperty("identifier"), is(nullValue())); assertThat(graph.getNodeAt("/segmentTestUuids/node1[2]").getProperty("identifier").getFirstValue().toString(), @@ -1092,7 +1094,7 @@ graph.move("/node2").before("/node2"); assertThat(graph.getChildren().of("/"), hasChildren(segment("node1"), segment("node2"), segment("node3"))); - } + } @Test public void shouldMoveNodes() { @@ -1859,7 +1861,7 @@ Stopwatch sw = new Stopwatch(); boolean batch = true; createSubgraph(graph, initialPath, depth, numChildrenPerNode, numPropertiesPerNode, batch, sw, System.out, null); - + assertThat(graph.getChildren().of("/"), hasChildren(segment("node1"))); Subgraph subgraph = graph.getSubgraphOfDepth(2).at("/"); assertThat(subgraph, is(notNullValue())); @@ -1888,7 +1890,7 @@ Stopwatch sw = new Stopwatch(); boolean batch = true; createSubgraph(graph, initialPath, depth, numChildrenPerNode, numPropertiesPerNode, batch, sw, System.out, null); - + assertThat(graph.getChildren().of("/"), hasChildren(segment("node1"))); graph.addValue("foo").andValue("bar").to("newProperty").on("node1"); @@ -1912,7 +1914,7 @@ Stopwatch sw = new Stopwatch(); boolean batch = true; createSubgraph(graph, initialPath, depth, numChildrenPerNode, numPropertiesPerNode, batch, sw, System.out, null); - + assertThat(graph.getChildren().of("/"), hasChildren(segment("node1"))); graph.removeValue("The quick brown fox jumped over the moon. What? ").andValue("bar").from("property1").on("node1"); @@ -1934,7 +1936,7 @@ Stopwatch sw = new Stopwatch(); boolean batch = true; createSubgraph(graph, initialPath, depth, numChildrenPerNode, numPropertiesPerNode, batch, sw, System.out, null); - + assertThat(graph.getChildren().of("/"), hasChildren(segment("node1"))); graph.removeValue("The quick brown fox jumped over the moon. What? ").from("noSuchProperty").on("node1"); Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrI18n.java =================================================================== --- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrI18n.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrI18n.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -96,6 +96,7 @@ public static I18n unableToSaveBranchBecauseChangesDependOnChangesToNodesOutsideOfBranch; public static I18n allPropertyValuesMustHaveSameType; public static I18n cannotRemoveNodeFromClone; + public static I18n cannotRemoveNodeFromCloneDueToChangesInSession; public static I18n cannotRemoveParentNodeOfTarget; public static I18n invalidPropertyType; Modified: trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java =================================================================== --- trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-jcr/src/main/java/org/jboss/dna/jcr/JcrWorkspace.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -346,6 +346,12 @@ String path = node.getPath().getString(context.getNamespaceRegistry()); throw new ConstraintViolationException(JcrI18n.cannotRemoveNodeFromClone.text(path, uuid)); } + // Check whether the node has any local changes ... + if (node.isChanged(true)) { + // This session has changes on nodes that will be removed as a result of the clone ... + String path = node.getPath().getString(context.getNamespaceRegistry()); + throw new RepositoryException(JcrI18n.cannotRemoveNodeFromCloneDueToChangesInSession.text(path, uuid)); + } } } } Modified: trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/JcrI18n.properties =================================================================== --- trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/JcrI18n.properties 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-jcr/src/main/resources/org/jboss/dna/jcr/JcrI18n.properties 2009-08-28 15:26:27 UTC (rev 1180) @@ -86,6 +86,7 @@ unableToSaveBranchBecauseChangesDependOnChangesToNodesOutsideOfBranch = Unable to save "{0}" in workspace "{1}" because it contains changes that depend on changes to nodes outside of this branch allPropertyValuesMustHaveSameType = All values of property "{0}" on node "{3}" in workspace "{4}" must all be {2} values (values were: {1}) cannotRemoveNodeFromClone = The node at "{0}" with UUID "{1}" exists in the current workspace but cannot be removed because it is a mandatory child node +cannotRemoveNodeFromCloneDueToChangesInSession = The node at "{0}" with UUID "{1}" already exists in the current workspace and would be removed by the clone, but that node has been changed within this session and therefore cannot be removed cannotRemoveParentNodeOfTarget = The node at "{0}" with UUID "{1}" is a parent of the target node for this operation "{2}" invalidPropertyType = Invalid property type: {0} Modified: trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java =================================================================== --- trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/dna-jcr/src/test/java/org/jboss/dna/jcr/JcrTckTest.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -65,6 +65,7 @@ import org.apache.jackrabbit.test.api.SetValueValueFormatExceptionTest; import org.apache.jackrabbit.test.api.SetValueVersionExceptionTest; import org.apache.jackrabbit.test.api.ValueFactoryTest; +import org.apache.jackrabbit.test.api.WorkspaceCloneReferenceableTest; import org.apache.jackrabbit.test.api.WorkspaceCloneSameNameSibsTest; import org.apache.jackrabbit.test.api.WorkspaceCloneVersionableTest; import org.apache.jackrabbit.test.api.WorkspaceCopyBetweenWorkspacesReferenceableTest; @@ -74,6 +75,7 @@ import org.apache.jackrabbit.test.api.WorkspaceCopyReferenceableTest; import org.apache.jackrabbit.test.api.WorkspaceCopySameNameSibsTest; import org.apache.jackrabbit.test.api.WorkspaceCopyVersionableTest; +import org.apache.jackrabbit.test.api.WorkspaceMoveReferenceableTest; import org.apache.jackrabbit.test.api.WorkspaceMoveSameNameSibsTest; import org.apache.jackrabbit.test.api.WorkspaceMoveVersionableTest; @@ -230,7 +232,7 @@ addTestSuite(NodeCanAddMixinTest.class); addTestSuite(NodeRemoveMixinTest.class); - // dna-466 addTestSuite(WorkspaceCloneReferenceableTest.class); + addTestSuite(WorkspaceCloneReferenceableTest.class); addTestSuite(WorkspaceCloneSameNameSibsTest.class); // addTestSuite(WorkspaceCloneTest.class); addTestSuite(WorkspaceCloneVersionableTest.class); @@ -242,7 +244,7 @@ addTestSuite(WorkspaceCopySameNameSibsTest.class); // addTestSuite(WorkspaceCopyTest.class); addTestSuite(WorkspaceCopyVersionableTest.class); - // dna-466 addTestSuite(WorkspaceMoveReferenceableTest.class); + addTestSuite(WorkspaceMoveReferenceableTest.class); addTestSuite(WorkspaceMoveSameNameSibsTest.class); // addTestSuite(WorkspaceMoveTest.class); addTestSuite(WorkspaceMoveVersionableTest.class); Modified: trunk/extensions/dna-connector-federation/src/main/java/org/jboss/dna/connector/federation/FederatingRequestProcessor.java =================================================================== --- trunk/extensions/dna-connector-federation/src/main/java/org/jboss/dna/connector/federation/FederatingRequestProcessor.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/extensions/dna-connector-federation/src/main/java/org/jboss/dna/connector/federation/FederatingRequestProcessor.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -325,8 +325,8 @@ } // Delete in the cache ... - DeleteBranchRequest cacheRequest = new DeleteBranchRequest(request.at(), - workspace.getCacheProjection().getWorkspaceName()); + DeleteBranchRequest cacheRequest = new DeleteBranchRequest(request.at(), workspace.getCacheProjection() + .getWorkspaceName()); executeInCache(cacheRequest, workspace); } @@ -380,8 +380,8 @@ } // Delete from the cache the parent of the new location ... - DeleteBranchRequest cacheRequest = new DeleteBranchRequest(request.into(), - fromWorkspace.getCacheProjection().getWorkspaceName()); + DeleteBranchRequest cacheRequest = new DeleteBranchRequest(request.into(), fromWorkspace.getCacheProjection() + .getWorkspaceName()); executeInCache(cacheRequest, fromWorkspace); } @@ -433,11 +433,14 @@ } else { request.setActualLocations(fromProjection.convertToRepository(sourceRequest.getActualLocationBefore()), intoProjection.convertToRepository(sourceRequest.getActualLocationAfter())); + if (sourceRequest.removeExisting()) { + request.setRemovedNodes(Collections.unmodifiableSet(intoProjection.convertToRepository(sourceRequest.getRemovedNodes()))); + } } // Delete from the cache the parent of the new location ... - DeleteBranchRequest cacheRequest = new DeleteBranchRequest(request.into(), - fromWorkspace.getCacheProjection().getWorkspaceName()); + DeleteBranchRequest cacheRequest = new DeleteBranchRequest(request.into(), fromWorkspace.getCacheProjection() + .getWorkspaceName()); executeInCache(cacheRequest, fromWorkspace); } @@ -484,8 +487,8 @@ intoProjection.convertToRepository(sourceRequest.getActualLocationAfter())); } // Delete from the cache ... - DeleteBranchRequest cacheRequest = new DeleteBranchRequest(request.from(), - workspace.getCacheProjection().getWorkspaceName()); + DeleteBranchRequest cacheRequest = new DeleteBranchRequest(request.from(), workspace.getCacheProjection() + .getWorkspaceName()); executeInCache(cacheRequest, workspace); // Mark the new parent node as being expired ... cacheRequest = new DeleteBranchRequest(request.into(), workspace.getCacheProjection().getWorkspaceName()); @@ -520,8 +523,8 @@ } // Update the cache ... - UpdatePropertiesRequest cacheRequest = new UpdatePropertiesRequest(request.on(), - workspace.getCacheProjection().getWorkspaceName(), + UpdatePropertiesRequest cacheRequest = new UpdatePropertiesRequest(request.on(), workspace.getCacheProjection() + .getWorkspaceName(), request.properties()); executeInCache(cacheRequest, workspace); } @@ -598,7 +601,7 @@ this.pathInSource = pathInSource; } - protected Location convertToRepository( Location sourceLocation ) { + protected final Location convertToRepository( Location sourceLocation ) { assert sourceLocation != null; if (sourceLocation.hasPath()) { Set<Path> paths = projection.getPathsInRepository(sourceLocation.getPath(), pathFactory); @@ -607,6 +610,15 @@ } return sourceLocation; } + + protected Set<Location> convertToRepository( Set<Location> sourceLocations ) { + assert sourceLocations != null; + Set<Location> results = new HashSet<Location>(); + for (Location sourceLocation : sourceLocations) { + results.add(convertToRepository(sourceLocation)); + } + return results; + } } protected SingleProjection asSingleProjection( FederatedWorkspace federatedWorkspace, @@ -1206,9 +1218,9 @@ readable(registry, create.properties())); } else if (request instanceof UpdatePropertiesRequest) { UpdatePropertiesRequest update = (UpdatePropertiesRequest)request; - logger.trace(" updating {0} with properties {1}", - update.on().getString(registry), - readable(registry, update.properties().values())); + logger.trace(" updating {0} with properties {1}", update.on().getString(registry), readable(registry, + update.properties() + .values())); } else { logger.trace(" " + request.toString()); } @@ -1227,9 +1239,9 @@ readable(registry, create.properties())); } else if (request instanceof UpdatePropertiesRequest) { UpdatePropertiesRequest update = (UpdatePropertiesRequest)request; - logger.trace(" updating {0} with properties {1}", - update.on().getString(registry), - readable(registry, update.properties().values())); + logger.trace(" updating {0} with properties {1}", update.on().getString(registry), readable(registry, + update.properties() + .values())); } else { logger.trace(" " + request.toString()); } Modified: trunk/extensions/dna-connector-filesystem/src/main/java/org/jboss/dna/connector/filesystem/FileSystemRequestProcessor.java =================================================================== --- trunk/extensions/dna-connector-filesystem/src/main/java/org/jboss/dna/connector/filesystem/FileSystemRequestProcessor.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/extensions/dna-connector-filesystem/src/main/java/org/jboss/dna/connector/filesystem/FileSystemRequestProcessor.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -703,6 +703,7 @@ } request.setActualLocations(copy.getActualLocationBefore(), copy.getActualLocationAfter()); + request.setRemovedNodes(null); } /** Modified: trunk/extensions/dna-connector-store-jpa/src/main/java/org/jboss/dna/connector/store/jpa/model/basic/BasicRequestProcessor.java =================================================================== --- trunk/extensions/dna-connector-store-jpa/src/main/java/org/jboss/dna/connector/store/jpa/model/basic/BasicRequestProcessor.java 2009-08-27 14:04:45 UTC (rev 1179) +++ trunk/extensions/dna-connector-store-jpa/src/main/java/org/jboss/dna/connector/store/jpa/model/basic/BasicRequestProcessor.java 2009-08-28 15:26:27 UTC (rev 1180) @@ -961,7 +961,7 @@ oos.close(); } } - + // The new large values were recorded and associated with the properties entity during reserialization. // However, any values no longer used now need to be removed ... if (hadLargeValues) { @@ -1427,6 +1427,7 @@ logger.trace(request.toString()); Location actualFromLocation = null; Location actualToLocation = null; + Set<Location> removedLocations = null; try { // Find the workspaces ... WorkspaceEntity fromWorkspace = getExistingWorkspace(request.fromWorkspace(), request); @@ -1652,6 +1653,7 @@ } // Remove from the cache of children locations all entries for deleted nodes ... cache.removeBranch(intoWorkspaceId, deletedLocations.values()); + removedLocations = Collections.unmodifiableSet(new HashSet<Location>(deletedLocations.values())); } LargeValueEntity.deleteUnused(entities); } @@ -1668,6 +1670,7 @@ return; } request.setActualLocations(actualFromLocation, actualToLocation); + request.setRemovedNodes(removedLocations); recordChange(request); } @@ -1932,7 +1935,7 @@ } assert beforeEntity != null; - + Name childName = oldPath.getLastSegment().getName(); String childLocalName = fromEntity.getChildName(); NamespaceEntity ns = fromEntity.getChildNamespace(); @@ -2084,8 +2087,8 @@ } else { ActualLocation actualBeforeLocation = getActualLocation(workspace, beforeLocation); - ActualLocation actualIntoLocation = getActualLocation(workspace, - Location.create(beforeLocation.getPath().getParent())); + ActualLocation actualIntoLocation = getActualLocation(workspace, Location.create(beforeLocation.getPath() + .getParent())); actualNewLocation = moveNodeBefore(workspace, actualLocation, actualIntoLocation, actualBeforeLocation); } @@ -2429,7 +2432,7 @@ props.setData(baos.toByteArray()); props.setPropertyCount(numProperties); - + // Record the changes to the references ... if (refs != null && refs.hasWritten()) { for (Reference reference : refs.getWritten()) { @@ -2555,7 +2558,7 @@ } } Path fullPath = pathFactory.createAbsolutePath(segments); - Location newLocation = original.with(fullPath); + Location newLocation = original.with(fullPath); cache.addNewNode(workspaceId, newLocation); return new ActualLocation(newLocation, nodeUuidString, originalEntity); } @@ -2578,13 +2581,13 @@ if (cachedParent != null) { // We know the UUID of the parent, so we can find the child a little faster ... ChildEntity child = findByPathSegment(workspaceId, cachedParent.getUuid().toString(), path.getLastSegment()); - + // If there is no matching child, throw an exception if (child == null) { // Could not find the node given the supplied path, so find the lowest path that does exist ... throw new PathNotFoundException(original, cachedParent.getPath(), JpaConnectorI18n.nodeDoesNotExist.text(path)); } - + uuidString = child.getId().getChildUuidString(); Location newLocation = original.with(UUID.fromString(uuidString)); cache.addNewNode(workspaceId, newLocation); @@ -2614,7 +2617,8 @@ return new ActualLocation(newLocation, uuidString, child); } - protected ChildEntity findNode( long workspaceId, String uuidString) { + protected ChildEntity findNode( long workspaceId, + String uuidString ) { Query query = entities.createNamedQuery("ChildEntity.findByChildUuid"); query.setParameter("workspaceId", workspaceId); query.setParameter("childUuidString", uuidString); @@ -2625,7 +2629,7 @@ return null; } } - + /** * Find the node with the supplied path segment that is a child of the supplied parent. *