Index: modeshape-graph/src/main/java/org/modeshape/graph/GraphI18n.java =================================================================== --- modeshape-graph/src/main/java/org/modeshape/graph/GraphI18n.java (revision 2460) +++ modeshape-graph/src/main/java/org/modeshape/graph/GraphI18n.java (working copy) @@ -100,6 +100,7 @@ public final class GraphI18n { public static I18n errorImportingContent; public static I18n unableToFindRepositorySourceWithName; public static I18n nodeAlreadyExistsWithUuid; + public static I18n nodeDoesNotExistWithUuid; public static I18n couldNotAcquireLock; public static I18n errorNotifyingObserver; Index: modeshape-graph/src/main/java/org/modeshape/graph/session/GraphSession.java =================================================================== --- modeshape-graph/src/main/java/org/modeshape/graph/session/GraphSession.java (revision 2460) +++ modeshape-graph/src/main/java/org/modeshape/graph/session/GraphSession.java (working copy) @@ -112,6 +112,12 @@ public class GraphSession { * A map that records how the changes to a node are dependent upon other nodes. */ protected final Map changeDependencies = new HashMap(); + /** + * A set that records the UUIDs of the nodes that have been deleted from this session (via the {@link #operations}) but not + * yet {@link #save() saved}. This is used to know whether a node has been locally removed to prevent reloading the node from + * the persistent store. + */ + protected final Set deletedNodes = new HashSet(); private LinkedList requests; private BatchRequestBuilder requestBuilder; @@ -260,11 +266,26 @@ public class GraphSession { } } + // Has this node already been deleted by this session (but not yet committed)? + if (this.deletedNodes.contains(uuid)) { + String msg = GraphI18n.nodeDoesNotExistWithUuid.text(uuid, workspaceName); + throw new PathNotFoundException(location, this.root.getPath(), msg); + } + // Query for the actual location ... location = store.getNodeAt(location).getLocation(); } assert location.hasPath(); - return findNodeWith(null, location.getPath()); + Node result = findNodeWith(null, location.getPath()); + if (uuid != null) { + // Check that the input UUID matches that of the result ... + UUID actualUuid = result.getLocation().getUuid(); + if (!uuid.equals(actualUuid)) { + String msg = GraphI18n.nodeDoesNotExistWithUuid.text(uuid, workspaceName); + throw new PathNotFoundException(location, this.root.getPath(), msg); + } + } + return result; } private UUID uuidFor( Location location ) { @@ -957,6 +978,9 @@ public class GraphSession { throw new RepositorySourceException(e.getLocalizedMessage(), e); } + // Clear out the record of which nodes were deleted in that batch ... + this.deletedNodes.clear(); + // Create a new batch for future operations ... // LinkedList oldRequests = this.requests; this.requests = new LinkedList(); @@ -1097,7 +1121,12 @@ public class GraphSession { */ protected void recordDelete( Node node ) { // Record the operation ... - operations.delete(node.getLocation()); + Location location = node.getLocation(); + operations.delete(location); + UUID nodeUuid = uuidFor(location); + if (nodeUuid != null) { + deletedNodes.add(nodeUuid); + } // Fix the cache's state ... nodes.remove(node.getNodeId()); changeDependencies.remove(node.getNodeId()); Index: modeshape-graph/src/main/resources/org/modeshape/graph/GraphI18n.properties =================================================================== --- modeshape-graph/src/main/resources/org/modeshape/graph/GraphI18n.properties (revision 2460) +++ modeshape-graph/src/main/resources/org/modeshape/graph/GraphI18n.properties (working copy) @@ -88,6 +88,7 @@ aliasesMappedToRealNamespacesButWereNotRegisteredInAliasNamespace = One or more errorImportingContent = Error importing {0} content from {1} unableToFindRepositorySourceWithName = Unable to find a repository source named "{0}" nodeAlreadyExistsWithUuid = A node with UUID "{0}" already exists at path "{1}" in workspace "{2}" +nodeDoesNotExistWithUuid = A node with UUID "{0}" has been locally removed from workspace "{1}" couldNotAcquireLock = Could not acquire lock on the node at "{0}" in workspace "{1}" errorNotifyingObserver = Error notifying observer: {0} Index: modeshape-integration-tests/src/test/java/org/modeshape/test/integration/AbstractModeShapeTest.java =================================================================== --- modeshape-integration-tests/src/test/java/org/modeshape/test/integration/AbstractModeShapeTest.java (revision 2460) +++ modeshape-integration-tests/src/test/java/org/modeshape/test/integration/AbstractModeShapeTest.java (working copy) @@ -32,11 +32,8 @@ import java.io.IOException; import java.io.InputStream; import java.net.URL; import java.util.Arrays; -import java.util.Collections; import java.util.HashMap; import java.util.HashSet; -import java.util.LinkedList; -import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.TimeUnit; @@ -45,8 +42,6 @@ import javax.jcr.Node; import javax.jcr.NodeIterator; import javax.jcr.PathNotFoundException; import javax.jcr.Property; -import javax.jcr.PropertyIterator; -import javax.jcr.PropertyType; import javax.jcr.RepositoryException; import javax.jcr.Session; import javax.jcr.Value; @@ -58,7 +53,6 @@ import net.jcip.annotations.Immutable; import org.junit.After; import org.junit.Before; import org.modeshape.common.util.CheckArg; -import org.modeshape.common.util.StringUtil; import org.modeshape.jcr.JcrConfiguration; import org.modeshape.jcr.JcrEngine; import org.modeshape.jcr.JcrRepository; @@ -594,82 +588,7 @@ public abstract class AbstractModeShapeTest { int depthOfSubgraph, int maxDepthOfSubgraph ) throws RepositoryException { if (!print) return; - int currentDepth = node.getDepth() - depthOfSubgraph + 1; - if (currentDepth > maxDepthOfSubgraph) return; - if (lead == null) lead = ""; - String nodeLead = lead + StringUtil.createString(' ', (currentDepth - 1) * 2); - - StringBuilder sb = new StringBuilder(); - sb.append(nodeLead); - if (node.getDepth() == 0) { - sb.append("/"); - } else { - sb.append(node.getName()); - if (node.getIndex() != 1) { - sb.append('[').append(node.getIndex()).append(']'); - } - } - sb.append(" jcr:primaryType=" + node.getPrimaryNodeType().getName()); - boolean referenceable = false; - if (node.getMixinNodeTypes().length != 0) { - sb.append(" jcr:mixinTypes=["); - boolean first = true; - for (NodeType mixin : node.getMixinNodeTypes()) { - if (first) first = false; - else sb.append(','); - sb.append(mixin.getName()); - if (mixin.getName().equals("mix:referenceable")) referenceable = true; - } - sb.append(']'); - } - if (referenceable) { - sb.append(" jcr:uuid=" + node.getIdentifier()); - } - System.out.println(sb); - - List propertyNames = new LinkedList(); - for (PropertyIterator iter = node.getProperties(); iter.hasNext();) { - Property property = iter.nextProperty(); - String name = property.getName(); - if (name.equals("jcr:primaryType") || name.equals("jcr:mixinTypes") || name.equals("jcr:uuid")) continue; - propertyNames.add(property.getName()); - } - Collections.sort(propertyNames); - for (String propertyName : propertyNames) { - Property property = node.getProperty(propertyName); - sb = new StringBuilder(); - sb.append(nodeLead).append(" - ").append(propertyName).append('='); - boolean binary = property.getType() == PropertyType.BINARY; - if (property.isMultiple()) { - sb.append('['); - boolean first = true; - for (Value value : property.getValues()) { - if (first) first = false; - else sb.append(','); - if (binary) { - sb.append(value.getBinary()); - } else { - sb.append(value.getString()); - } - } - sb.append(']'); - } else { - Value value = property.getValue(); - if (binary) { - sb.append(value.getBinary()); - } else { - sb.append(value.getString()); - } - } - System.out.println(sb); - } - - if (currentDepth < maxDepthOfSubgraph) { - for (NodeIterator iter = node.getNodes(); iter.hasNext();) { - Node child = iter.nextNode(); - printSubgraph(child, lead, depthOfSubgraph, maxDepthOfSubgraph); - } - } + tools.printSubgraph(node, lead, depthOfSubgraph, maxDepthOfSubgraph); } protected void printChildren( Node node ) throws RepositoryException { Index: modeshape-jcr/src/main/java/org/modeshape/jcr/JcrTools.java =================================================================== --- modeshape-jcr/src/main/java/org/modeshape/jcr/JcrTools.java (revision 2460) +++ modeshape-jcr/src/main/java/org/modeshape/jcr/JcrTools.java (working copy) @@ -30,14 +30,23 @@ import java.io.IOException; import java.io.InputStream; import java.net.URL; import java.util.Calendar; +import java.util.Collections; +import java.util.LinkedList; +import java.util.List; import javax.jcr.Binary; import javax.jcr.Node; import javax.jcr.NodeIterator; import javax.jcr.PathNotFoundException; +import javax.jcr.Property; +import javax.jcr.PropertyIterator; +import javax.jcr.PropertyType; import javax.jcr.Repository; import javax.jcr.RepositoryException; import javax.jcr.Session; +import javax.jcr.Value; +import javax.jcr.nodetype.NodeType; import org.modeshape.common.util.CheckArg; +import org.modeshape.common.util.StringUtil; import org.modeshape.graph.ExecutionContext; import org.modeshape.graph.mimetype.MimeTypeDetector; @@ -442,4 +451,127 @@ public class JcrTools { return findOrCreateNode(parent, name, nodeType, nodeType); } + /** + * Load the subgraph below this node, and print it to System.out if printing is enabled. + * + * @param node the root of the subgraph + * @throws RepositoryException + */ + public void printSubgraph( Node node ) throws RepositoryException { + printSubgraph(node, Integer.MAX_VALUE); + } + + /** + * Load the subgraph below this node, and print it to System.out if printing is enabled. + * + * @param node the root of the subgraph + * @param maxDepth the maximum depth of the subgraph that should be printed + * @throws RepositoryException + */ + public void printSubgraph( Node node, + int maxDepth ) throws RepositoryException { + printSubgraph(node, " ", node.getDepth(), maxDepth); + } + + /** + * Print this node and its properties to System.out if printing is enabled. + * + * @param node the node to be printed + * @throws RepositoryException + */ + public void printNode( Node node ) throws RepositoryException { + printSubgraph(node, " ", node.getDepth(), 1); + } + + /** + * Load the subgraph below this node, and print it to System.out if printing is enabled. + * + * @param node the root of the subgraph + * @param lead the string that each line should begin with; may be null if there is no such string + * @param depthOfSubgraph the depth of this subgraph's root node + * @param maxDepthOfSubgraph the maximum depth of the subgraph that should be printed + * @throws RepositoryException + */ + public void printSubgraph( Node node, + String lead, + int depthOfSubgraph, + int maxDepthOfSubgraph ) throws RepositoryException { + int currentDepth = node.getDepth() - depthOfSubgraph + 1; + if (currentDepth > maxDepthOfSubgraph) return; + if (lead == null) lead = ""; + String nodeLead = lead + StringUtil.createString(' ', (currentDepth - 1) * 2); + + StringBuilder sb = new StringBuilder(); + sb.append(nodeLead); + if (node.getDepth() == 0) { + sb.append("/"); + } else { + sb.append(node.getName()); + if (node.getIndex() != 1) { + sb.append('[').append(node.getIndex()).append(']'); + } + } + sb.append(" jcr:primaryType=" + node.getPrimaryNodeType().getName()); + boolean referenceable = false; + if (node.getMixinNodeTypes().length != 0) { + sb.append(" jcr:mixinTypes=["); + boolean first = true; + for (NodeType mixin : node.getMixinNodeTypes()) { + if (first) first = false; + else sb.append(','); + sb.append(mixin.getName()); + if (mixin.getName().equals("mix:referenceable")) referenceable = true; + } + sb.append(']'); + } + if (referenceable) { + sb.append(" jcr:uuid=" + node.getIdentifier()); + } + System.out.println(sb); + + List propertyNames = new LinkedList(); + for (PropertyIterator iter = node.getProperties(); iter.hasNext();) { + Property property = iter.nextProperty(); + String name = property.getName(); + if (name.equals("jcr:primaryType") || name.equals("jcr:mixinTypes") || name.equals("jcr:uuid")) continue; + propertyNames.add(property.getName()); + } + Collections.sort(propertyNames); + for (String propertyName : propertyNames) { + Property property = node.getProperty(propertyName); + sb = new StringBuilder(); + sb.append(nodeLead).append(" - ").append(propertyName).append('='); + boolean binary = property.getType() == PropertyType.BINARY; + if (property.isMultiple()) { + sb.append('['); + boolean first = true; + for (Value value : property.getValues()) { + if (first) first = false; + else sb.append(','); + if (binary) { + sb.append(value.getBinary()); + } else { + sb.append(value.getString()); + } + } + sb.append(']'); + } else { + Value value = property.getValue(); + if (binary) { + sb.append(value.getBinary()); + } else { + sb.append(value.getString()); + } + } + System.out.println(sb); + } + + if (currentDepth < maxDepthOfSubgraph) { + for (NodeIterator iter = node.getNodes(); iter.hasNext();) { + Node child = iter.nextNode(); + printSubgraph(child, lead, depthOfSubgraph, maxDepthOfSubgraph); + } + } + } + } Index: modeshape-jcr/src/test/java/org/modeshape/jcr/ImportExportTest.java =================================================================== --- modeshape-jcr/src/test/java/org/modeshape/jcr/ImportExportTest.java (revision 2460) +++ modeshape-jcr/src/test/java/org/modeshape/jcr/ImportExportTest.java (working copy) @@ -24,12 +24,17 @@ package org.modeshape.jcr; import static org.hamcrest.core.Is.is; +import static org.hamcrest.core.IsNot.not; +import static org.hamcrest.core.IsNull.notNullValue; import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; import java.io.ByteArrayInputStream; import java.io.ByteArrayOutputStream; +import java.io.InputStream; import java.util.Collections; import javax.jcr.ImportUUIDBehavior; import javax.jcr.Node; +import javax.jcr.PathNotFoundException; import org.junit.After; import org.junit.Before; import org.junit.Ignore; @@ -59,10 +64,13 @@ public class ImportExportTest { private InMemoryRepositorySource source; private JcrSession session; private JcrRepository repository; + @SuppressWarnings( "unused" ) + private JcrTools tools; @Before public void beforeEach() throws Exception { MockitoAnnotations.initMocks(this); + this.tools = new JcrTools(); String workspaceName = "workspace1"; @@ -86,7 +94,7 @@ public class ImportExportTest { repository = new JcrRepository(context, connectionFactory, "unused", new MockObservable(), null, null, null); - SecurityContext mockSecurityContext = new MockSecurityContext("testuser", Collections.singleton(ModeShapeRoles.READWRITE)); + SecurityContext mockSecurityContext = new MockSecurityContext("testuser", Collections.singleton(ModeShapeRoles.ADMIN)); session = (JcrSession)repository.login(new JcrSecurityContextCredentials(mockSecurityContext)); } @@ -177,4 +185,146 @@ public class ImportExportTest { newSourceNode.getNode(BAD_CHARACTER_STRING); assertThat(newSourceNode.getProperty("badcharacters").getString(), is(BAD_CHARACTER_STRING)); } + + protected void importFile( String importIntoPath, + String resourceName, + int importBehavior ) throws Exception { + // Import the car content ... + InputStream stream = getClass().getClassLoader().getResourceAsStream(resourceName); + assertThat(stream, is(notNullValue())); + try { + session.importXML(importIntoPath, stream, importBehavior); // shouldn't exist yet + } finally { + stream.close(); + } + } + + protected Node assertNode( String path ) throws Exception { + Node node = session.getNode(path); + assertThat(node, is(notNullValue())); + return node; + } + + protected void assertNoNode( String path ) throws Exception { + try { + session.getNode(path); + fail("Did not expect to find node at \"" + path + "\""); + } catch (PathNotFoundException e) { + // expected + } + } + + @Test + public void shouldImportSystemViewWithUuidsAfterNodesWithSameUuidsAreDeletedInSessionAndSaved() throws Exception { + // Register the Cars node types ... + CndNodeTypeReader reader = new CndNodeTypeReader(session); + reader.read("cars.cnd"); + session.getWorkspace().getNodeTypeManager().registerNodeTypes(reader.getNodeTypeDefinitions(), true); + + // Create the node under which the content will be imported ... + session.getRootNode().addNode("/someNode"); + session.save(); + + // Import the car content ... + importFile("/someNode", "io/cars-system-view-with-uuids.xml", ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW); + session.save(); + // tools.printSubgraph(assertNode("/")); + + // Now delete the '/someNode/Cars' node (which is everything that was imported) ... + Node cars = assertNode("/someNode/Cars"); + assertThat(cars.getIdentifier(), is("e41075cb-a09a-4910-87b1-90ce8b4ca9dd")); + assertNoNode("/someNode/Cars[2]"); + assertNoNode("/someNode[2]"); + cars.remove(); + session.save(); + + // Now import again ... + importFile("/someNode", "io/cars-system-view-with-uuids.xml", ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW); + session.save(); + + // Verify the same Cars node exists ... + cars = assertNode("/someNode/Cars"); + assertThat(cars.getIdentifier(), is("e41075cb-a09a-4910-87b1-90ce8b4ca9dd")); + assertNoNode("/someNode/Cars[2]"); + assertNoNode("/someNode[2]"); + } + + @Test + public void shouldImportSystemViewWithUuidsAfterNodesWithSameUuidsAreDeletedInSessionButNotSaved() throws Exception { + // Register the Cars node types ... + CndNodeTypeReader reader = new CndNodeTypeReader(session); + reader.read("cars.cnd"); + session.getWorkspace().getNodeTypeManager().registerNodeTypes(reader.getNodeTypeDefinitions(), true); + + // Create the node under which the content will be imported ... + session.getRootNode().addNode("/someNode"); + session.save(); + + // Import the car content ... + importFile("/someNode", "io/cars-system-view-with-uuids.xml", ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW); + session.save(); + + // Now delete the '/someNode/Cars' node (which is everything that was imported) ... + Node cars = assertNode("/someNode/Cars"); + assertThat(cars.getIdentifier(), is("e41075cb-a09a-4910-87b1-90ce8b4ca9dd")); + assertNoNode("/someNode/Cars[2]"); + assertNoNode("/someNode[2]"); + cars.remove(); + // session.save(); // DO NOT SAVE + + // Now import again ... + importFile("/someNode", "io/cars-system-view-with-uuids.xml", ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW); + session.save(); + + // Verify the same Cars node exists ... + cars = assertNode("/someNode/Cars"); + assertThat(cars.getIdentifier(), is("e41075cb-a09a-4910-87b1-90ce8b4ca9dd")); + assertNoNode("/someNode/Cars[2]"); + assertNoNode("/someNode[2]"); + } + + @Test + public void shouldImportSystemViewWithUuidsIntoDifferentSpotAfterNodesWithSameUuidsAreDeletedInSessionButNotSaved() + throws Exception { + // Register the Cars node types ... + CndNodeTypeReader reader = new CndNodeTypeReader(session); + reader.read("cars.cnd"); + session.getWorkspace().getNodeTypeManager().registerNodeTypes(reader.getNodeTypeDefinitions(), true); + + // Create the node under which the content will be imported ... + Node someNode = session.getRootNode().addNode("/someNode"); + session.getRootNode().addNode("/otherNode"); + session.save(); + + // Import the car content ... + importFile("/someNode", "io/cars-system-view-with-uuids.xml", ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW); + session.save(); + + // Now delete the '/someNode/Cars' node (which is everything that was imported) ... + Node cars = assertNode("/someNode/Cars"); + assertThat(cars.getIdentifier(), is("e41075cb-a09a-4910-87b1-90ce8b4ca9dd")); + assertNoNode("/someNode/Cars[2]"); + assertNoNode("/someNode[2]"); + cars.remove(); + + // Now create a node at the same spot as cars, but with a different UUID ... + Node newCars = someNode.addNode("Cars"); + assertThat(newCars.getIdentifier(), is(not("e41075cb-a09a-4910-87b1-90ce8b4ca9dd"))); + + // session.save(); // DO NOT SAVE + + // Now import again ... + importFile("/otherNode", "io/cars-system-view-with-uuids.xml", ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW); + session.save(); + + // Verify the same Cars node exists ... + cars = assertNode("/otherNode/Cars"); + assertThat(cars.getIdentifier(), is("e41075cb-a09a-4910-87b1-90ce8b4ca9dd")); + + // Make sure some duplicate nodes didn't show up ... + assertNoNode("/sameNode/Cars[2]"); + assertNoNode("/sameNode[2]"); + assertNoNode("/otherNode/Cars[2]"); + assertNoNode("/otherNode[2]"); + } }