[dna-commits] DNA SVN: r414 - in trunk: connectors/dna-connector-jbosscache/src/test/java/org/jboss/dna/connector/jbosscache and 4 other directories.

Author: rhauch Date: 2008-08-11 16:30:45 -0400 (Mon, 11 Aug 2008) New Revision: 414 Modified: trunk/connectors/dna-connector-jbosscache/src/main/java/org/jboss/dna/connector/jbosscache/JBossCacheConnection.java trunk/connectors/dna-connector-jbosscache/src/test/java/org/jboss/dna/connector/jbosscache/JBossCacheConnectionTest.java trunk/connectors/dna-connector-jbosscache/src/test/resources/log4j.properties trunk/dna-spi/src/main/java/org/jboss/dna/spi/graph/commands/executor/AbstractCommandExecutor.java trunk/dna-spi/src/main/java/org/jboss/dna/spi/graph/commands/impl/BasicCreateNodeCommand.java trunk/dna-spi/src/test/java/org/jboss/dna/spi/graph/commands/executor/AbstractCommandExecutorTest.java Log: DNA-83 - Federate content from JBoss Cache instance(s) http://jira.jboss.com/jira/browse/DNA-83 Addressed the need to support same-name siblings, which is more difficult to do with JBoss Cache, since children are not kept in any order. The code now maintains for any node in JBoss Cache a Path.Segment[] in a property, which stores it's ordered list of children. This is the most memory-efficient way to do this (that I could think of), since the actual Path.Segment objects should already be stored in the Fqn for each node, and no indexes need be maintained. So the only overhead is that associated with the array (roughly one reference per child). (Another option considered were wrapping the Path.Segment implementation with another implementation of Path.Segment that also stored the index, but this required a reference plus an integer for each child.) Found a bug in the AbstractCommandExecutor's handling of GetNodeCommand, which resulted in the GetPropertiesCommand and GetChildrenCommand being executed twice. Modified: trunk/connectors/dna-connector-jbosscache/src/main/java/org/jboss/dna/connector/jbosscache/JBossCacheConnection.java =================================================================== --- trunk/connectors/dna-connector-jbosscache/src/main/java/org/jboss/dna/connector/jbosscache/JBossCacheConnection.java 2008-08-10 18:43:31 UTC (rev 413) +++ trunk/connectors/dna-connector-jbosscache/src/main/java/org/jboss/dna/connector/jbosscache/JBossCacheConnection.java 2008-08-11 20:30:45 UTC (rev 414) @@ -21,6 +21,8 @@ */ package org.jboss.dna.connector.jbosscache; +import java.util.ArrayList; +import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -31,6 +33,8 @@ import org.jboss.cache.Cache; import org.jboss.cache.Fqn; import org.jboss.cache.Node; +import org.jboss.dna.common.util.Logger; +import org.jboss.dna.common.util.StringUtil; import org.jboss.dna.spi.ExecutionContext; import org.jboss.dna.spi.cache.CachePolicy; import org.jboss.dna.spi.connector.RepositoryConnection; @@ -59,6 +63,8 @@ import org.jboss.dna.spi.graph.commands.executor.CommandExecutor; /** + * The repository connection to a JBoss Cache instance. + * * @author Randall Hauch */ public class JBossCacheConnection implements RepositoryConnection { @@ -173,7 +179,7 @@ return this.uuidPropertyName; } - protected Fqn<Path.Segment> getFullyQualifiedName( Path path ) { + protected Fqn<?> getFullyQualifiedName( Path path ) { assert path != null; return Fqn.fromList(path.getSegmentsList()); } @@ -184,20 +190,22 @@ * @param pathSegment the segment from which the fully qualified name is to be created * @return the relative fully-qualified name */ - protected Fqn<Path.Segment> getFullyQualifiedName( Path.Segment pathSegment ) { + protected Fqn<?> getFullyQualifiedName( Path.Segment pathSegment ) { assert pathSegment != null; return Fqn.fromElements(pathSegment); } + @SuppressWarnings( "unchecked" ) protected Path getPath( PathFactory factory, - Fqn<Path.Segment> fqn ) { - return factory.create(factory.createRootPath(), fqn.peekElements()); + Fqn<?> fqn ) { + List<Path.Segment> segments = (List<Path.Segment>)fqn.peekElements(); + return factory.create(factory.createRootPath(), segments); } protected Node<Name, Object> getNode( ExecutionContext context, Path path ) { // Look up the node with the supplied path ... - Fqn<Segment> fqn = getFullyQualifiedName(path); + Fqn<?> fqn = getFullyQualifiedName(path); Node<Name, Object> node = cache.getNode(fqn); if (node == null) { String nodePath = path.getString(context.getNamespaceRegistry()); @@ -223,12 +231,19 @@ protected int copyNode( Node<Name, Object> original, Node<Name, Object> newParent, boolean recursive, - Name uuidProperty ) { + Name uuidProperty, + ExecutionContext context ) { assert original != null; assert newParent != null; // Get or create the new node ... Segment name = (Segment)original.getFqn().getLastElement(); - Node<Name, Object> copy = newParent.addChild(getFullyQualifiedName(name)); + + // Update the children to account for same-name siblings. + // This not only updates the FQN of the child nodes, but it also sets the property that stores the + // the array of Path.Segment for the children (since the cache doesn't maintain order). + Path.Segment newSegment = updateChildList(newParent, name.getName(), context, true); + Node<Name, Object> copy = newParent.addChild(getFullyQualifiedName(newSegment)); + assert checkChildren(newParent); // Copy the properties ... copy.clearData(); copy.putAll(original.getData()); @@ -240,7 +255,7 @@ if (recursive) { // Loop over each child and call this method ... for (Node<Name, Object> child : original.getChildren()) { - numNodesCopied += copyNode(child, copy, true, uuidProperty); + numNodesCopied += copyNode(child, copy, true, uuidProperty, context); } } return numNodesCopied; @@ -256,90 +271,136 @@ * @param changedName the name that should be compared to the existing node siblings to determine whether the same-name * sibling indexes should be updated; may not be null * @param context the execution context; may not be null + * @param addChildWithName true if a new child with the supplied name is to be added to the children (but which does not yet + * exist in the node's children) + * @return the path segment for the new child, or null if <code>addChildWithName</code> was false */ - @SuppressWarnings( "unchecked" ) - protected void updateChildList( Node<Name, Object> parent, - Name changedName, - ExecutionContext context ) { + protected Path.Segment updateChildList( Node<Name, Object> parent, + Name changedName, + ExecutionContext context, + boolean addChildWithName ) { assert parent != null; assert changedName != null; assert context != null; Set<Node<Name, Object>> children = parent.getChildren(); - final int numChildren = children.size(); - if (numChildren == 0) return; + if (children.isEmpty() && !addChildWithName) return null; + // Go through the children, looking for any children with the same name as the 'changedName' List<ChildInfo> childrenWithChangedName = new LinkedList<ChildInfo>(); - Path.Segment[] childSegments = new Path.Segment[children.size()]; + Path.Segment[] childNames = (Path.Segment[])parent.get(JBossCacheLexicon.CHILD_PATH_SEGMENT_LIST); int index = 0; - for (Node<Name, Object> child : children) { - Path.Segment childSegment = (Path.Segment)child.getFqn().getLastElement(); - Name childName = childSegment.getName(); - if (childName.equals(changedName)) { - ChildInfo info = new ChildInfo(child.getFqn(), index); - childrenWithChangedName.add(info); + if (childNames != null) { + for (Path.Segment childName : childNames) { + if (childName.getName().equals(changedName)) { + ChildInfo info = new ChildInfo(childName, index); + childrenWithChangedName.add(info); + } + index++; } - childSegments[index++] = childSegment; } - // Go through the children with the same name as the 'changedName', making sure their indexes are correct ... + if (addChildWithName) { + // Make room for the new child at the end of the array ... + if (childNames == null) { + childNames = new Path.Segment[1]; + } else { + int numExisting = childNames.length; + Path.Segment[] newChildNames = new Path.Segment[numExisting + 1]; + System.arraycopy(childNames, 0, newChildNames, 0, numExisting); + childNames = newChildNames; + } + + // And add a child info for the new node ... + ChildInfo info = new ChildInfo(null, index); + childrenWithChangedName.add(info); + Path.Segment newSegment = context.getValueFactories().getPathFactory().createSegment(changedName); + childNames[index++] = newSegment; + } + assert childNames != null; + + // Now process the children with the same name, which may include a child info for the new node ... assert childrenWithChangedName.isEmpty() == false; if (childrenWithChangedName.size() == 1) { // The child should have no indexes ... ChildInfo child = childrenWithChangedName.get(0); - Fqn<Path.Segment> fqn = child.getFqn(); - Path.Segment segment = fqn.getLastElement(); - if (segment.hasIndex()) { - // Determine the new name and index ... + if (child.segment != null && child.segment.hasIndex()) { + // The existing child needs to have a new index .. Path.Segment newSegment = context.getValueFactories().getPathFactory().createSegment(changedName); // Replace the child with the correct FQN ... - changeNodeName(parent, fqn, newSegment, context); + changeNodeName(parent, child.segment, newSegment, context); // Change the segment in the child list ... - childSegments[child.getChildIndex()] = newSegment; + childNames[child.childIndex] = newSegment; } } else { // There is more than one child with the same name ... int i = 0; for (ChildInfo child : childrenWithChangedName) { - Fqn<Path.Segment> fqn = child.getFqn(); - Path.Segment childSegment = fqn.getLastElement(); - if (childSegment.getIndex() != i) { + if (child.segment != null) { // Determine the new name and index ... - Path.Segment newSegment = context.getValueFactories().getPathFactory().createSegment(changedName, i); + Path.Segment newSegment = context.getValueFactories().getPathFactory().createSegment(changedName, i + 1); // Replace the child with the correct FQN ... - changeNodeName(parent, fqn, newSegment, context); + changeNodeName(parent, child.segment, newSegment, context); // Change the segment in the child list ... - childSegments[child.getChildIndex()] = newSegment; + childNames[child.childIndex] = newSegment; + } else { + // Determine the new name and index ... + Path.Segment newSegment = context.getValueFactories().getPathFactory().createSegment(changedName, i + 1); + childNames[child.childIndex] = newSegment; } ++i; } } + // Record the list of children as a property on the parent ... // (Do this last, as it doesn't need to be done if there's an exception in the above logic) - parent.put(JBossCacheLexicon.CHILD_PATH_SEGMENT_LIST, childSegments); // replaces any existing value + Logger.getLogger(getClass()).trace("Updating child list of {0} to: {1}", + parent.getFqn(), + StringUtil.readableString(childNames)); + parent.put(JBossCacheLexicon.CHILD_PATH_SEGMENT_LIST, childNames); // replaces any existing value + + if (addChildWithName) { + // Return the segment for the new node ... + return childNames[childNames.length - 1]; + } + return null; } + protected boolean checkChildren( Node<Name, Object> parent ) { + Path.Segment[] childNamesProperty = (Path.Segment[])parent.get(JBossCacheLexicon.CHILD_PATH_SEGMENT_LIST); + Set<Object> childNames = parent.getChildrenNames(); + boolean result = true; + if (childNamesProperty.length != childNames.size()) result = false; + for (int i = 0; i != childNamesProperty.length; ++i) { + if (!childNames.contains(childNamesProperty[i])) result = false; + } + if (!result) { + List<Path.Segment> names = new ArrayList<Path.Segment>(); + for (Object name : childNames) { + names.add((Path.Segment)name); + } + Collections.sort(names); + // Logger.getLogger(getClass()).trace("Child list on {0} is: {1}", + // parent.getFqn(), + // StringUtil.readableString(childNamesProperty)); + // Logger.getLogger(getClass()).trace("Children of {0} is: {1}", parent.getFqn(), StringUtil.readableString(names)); + } + return result; + } + /** - * Utility class used by the {@link JBossCacheConnection#updateChildList(Node, Name, ExecutionContext)} method. + * Utility class used by the {@link JBossCacheConnection#updateChildList(Node, Name, ExecutionContext, boolean)} method. * * @author Randall Hauch */ private static class ChildInfo { - private final Fqn<Path.Segment> fqn; - private final int childIndex; + protected final Path.Segment segment; + protected final int childIndex; - protected ChildInfo( Fqn<Path.Segment> fqn, + protected ChildInfo( Path.Segment childSegment, int childIndex ) { - assert fqn != null; - this.fqn = fqn; + this.segment = childSegment; this.childIndex = childIndex; } - public int getChildIndex() { - return childIndex; - } - - public Fqn<Path.Segment> getFqn() { - return fqn; - } } /** @@ -351,20 +412,38 @@ * @param context */ protected void changeNodeName( Node<Name, Object> parent, - Fqn<Path.Segment> existing, + Path.Segment existing, Path.Segment newSegment, ExecutionContext context ) { assert parent != null; assert existing != null; assert newSegment != null; assert context != null; + + if (existing.equals(newSegment)) return; + Logger.getLogger(getClass()).trace("Renaming {0} to {1} under {2}", existing, newSegment, parent.getFqn()); + Node<Name, Object> existingChild = parent.getChild(existing); + assert existingChild != null; + + // JBoss Cache can move a node from one node to another node, but the move doesn't change the name; + // since you provide the FQN of the parent location, the name of the node cannot be changed. + // Therefore, to compensate, we need to create a new child, copy all of the data, move all of the child + // nodes of the old node, then remove the old node. + + // Create the new node ... + Node<Name, Object> newChild = parent.addChild(Fqn.fromElements(newSegment)); + Fqn<?> newChildFqn = newChild.getFqn(); + + // Copy the data ... + newChild.putAll(existingChild.getData()); + + // Move the children ... + for (Node<Name, Object> grandChild : existingChild.getChildren()) { + cache.move(grandChild.getFqn(), newChildFqn); + } + + // Remove the existing ... parent.removeChild(existing); - List<Path.Segment> elements = existing.peekElements(); - assert elements.size() > 0; - elements.set(elements.size() - 1, newSegment); - existing = Fqn.fromList(elements); - parent.addChild(existing); - } protected class Executor extends AbstractCommandExecutor { @@ -383,15 +462,15 @@ public void execute( CreateNodeCommand command ) { Path path = command.getPath(); Path parent = path.getAncestor(); - Fqn<Segment> childFqn = getFullyQualifiedName(path.getLastSegment()); // Look up the parent node, which must exist ... Node<Name, Object> parentNode = getNode(parent); - Node<Name, Object> node = parentNode.addChild(childFqn); // Update the children to account for same-name siblings. // This not only updates the FQN of the child nodes, but it also sets the property that stores the // the array of Path.Segment for the children (since the cache doesn't maintain order). - updateChildList(parentNode, path.getLastSegment().getName(), getExecutionContext()); + Path.Segment newSegment = updateChildList(parentNode, path.getLastSegment().getName(), getExecutionContext(), true); + Node<Name, Object> node = parentNode.addChild(Fqn.fromElements(newSegment)); + assert checkChildren(parentNode); // Add the UUID property (if required), which may be overwritten by a supplied property ... Name uuidPropertyName = getUuidPropertyName(getExecutionContext()); @@ -412,24 +491,19 @@ } } - @SuppressWarnings( "unchecked" ) @Override public void execute( GetChildrenCommand command ) { Node<Name, Object> node = getNode(command.getPath()); Name uuidPropertyName = getUuidPropertyName(getExecutionContext()); - List<Path.Segment> segments = node.getFqn().peekElements(); - segments.add(null); // Get the names of the children, using the child list ... Path.Segment[] childList = (Path.Segment[])node.get(JBossCacheLexicon.CHILD_PATH_SEGMENT_LIST); for (Path.Segment child : childList) { // We have the child segment, but we need the UUID property ... - segments.set(segments.size() - 1, child); // each iteration sets this last list element ... - Fqn<Path.Segment> fqn = Fqn.fromList(segments); - Node<Name, Object> childNode = node.getChild(fqn); - Object uuid = childNode.getData().get(uuidPropertyName); + Node<Name, Object> childNode = node.getChild(child); + Object uuid = childNode.get(uuidPropertyName); if (uuid == null) { uuid = generateUuid(); - childNode.getData().put(uuidPropertyName, uuid); + childNode.put(uuidPropertyName, uuid); } else { uuid = uuidFactory.create(uuid); } @@ -486,11 +560,7 @@ // Look up the new parent, which must exist ... Path newPath = command.getNewPath(); Node<Name, Object> newParent = getNode(newPath.getAncestor()); - copyNode(node, newParent, false, null); - // Update the children to account for same-name siblings. - // This not only updates the FQN of the child nodes, but it also sets the property that stores the - // the array of Path.Segment for the children (since the cache doesn't maintain order). - updateChildList(newParent, newPath.getLastSegment().getName(), getExecutionContext()); + copyNode(node, newParent, false, null, getExecutionContext()); } @Override @@ -499,11 +569,7 @@ // Look up the new parent, which must exist ... Path newPath = command.getNewPath(); Node<Name, Object> newParent = getNode(newPath.getAncestor()); - copyNode(node, newParent, true, null); - // Update the children to account for same-name siblings. - // This not only updates the FQN of the child nodes, but it also sets the property that stores the - // the array of Path.Segment for the children (since the cache doesn't maintain order). - updateChildList(newParent, newPath.getLastSegment().getName(), getExecutionContext()); + copyNode(node, newParent, true, null, getExecutionContext()); } @Override @@ -514,11 +580,8 @@ // Look up the new parent, which must exist ... Path newPath = command.getNewPath(); Node<Name, Object> newParent = getNode(newPath.getAncestor()); - copyNode(node, newParent, recursive, uuidProperty); - // Update the children to account for same-name siblings. - // This not only updates the FQN of the child nodes, but it also sets the property that stores the - // the array of Path.Segment for the children (since the cache doesn't maintain order). - updateChildList(newParent, newPath.getLastSegment().getName(), getExecutionContext()); + copyNode(node, newParent, recursive, uuidProperty, getExecutionContext()); + // Now delete the old node ... Node<Name, Object> oldParent = node.getParent(); boolean removed = oldParent.removeChild(node.getFqn().getLastElement()); Modified: trunk/connectors/dna-connector-jbosscache/src/test/java/org/jboss/dna/connector/jbosscache/JBossCacheConnectionTest.java =================================================================== --- trunk/connectors/dna-connector-jbosscache/src/test/java/org/jboss/dna/connector/jbosscache/JBossCacheConnectionTest.java 2008-08-10 18:43:31 UTC (rev 413) +++ trunk/connectors/dna-connector-jbosscache/src/test/java/org/jboss/dna/connector/jbosscache/JBossCacheConnectionTest.java 2008-08-11 20:30:45 UTC (rev 414) @@ -45,9 +45,14 @@ import org.jboss.dna.spi.connector.BasicExecutionContext; import org.jboss.dna.spi.connector.RepositorySourceListener; import org.jboss.dna.spi.graph.Name; +import org.jboss.dna.spi.graph.NameFactory; import org.jboss.dna.spi.graph.Path; import org.jboss.dna.spi.graph.PathFactory; import org.jboss.dna.spi.graph.PathNotFoundException; +import org.jboss.dna.spi.graph.Property; +import org.jboss.dna.spi.graph.PropertyFactory; +import org.jboss.dna.spi.graph.commands.impl.BasicCreateNodeCommand; +import org.jboss.dna.spi.graph.commands.impl.BasicGetNodeCommand; import org.junit.Before; import org.junit.Test; import org.mockito.MockitoAnnotations; @@ -63,6 +68,8 @@ private Cache<Name, Object> cache; private ExecutionContext context; private PathFactory pathFactory; + private NameFactory nameFactory; + private PropertyFactory propertyFactory; @Mock private JBossCacheSource source; @@ -72,9 +79,13 @@ context = new BasicExecutionContext(); context.getNamespaceRegistry().register(DnaLexicon.Namespace.PREFIX, DnaLexicon.Namespace.URI); pathFactory = context.getValueFactories().getPathFactory(); + propertyFactory = context.getPropertyFactory(); + nameFactory = context.getValueFactories().getNameFactory(); cacheFactory = new DefaultCacheFactory<Name, Object>(); cache = cacheFactory.createCache(); connection = new JBossCacheConnection(source, cache); + stub(source.getUuidPropertyName()).toReturn(DnaLexicon.UUID.getString(context.getNamespaceRegistry())); + stub(source.getName()).toReturn("the source name"); } @Test( expected = AssertionError.class ) @@ -164,18 +175,18 @@ @Test public void shouldCreateFullyQualifiedNodeOfPathSegmentsFromPath() { Path path = pathFactory.create("/a/b/c/d"); - Fqn<Path.Segment> fqn = connection.getFullyQualifiedName(path); + Fqn<?> fqn = connection.getFullyQualifiedName(path); assertThat(fqn.size(), is(4)); assertThat(fqn.isRoot(), is(false)); for (int i = 0; i != path.size(); ++i) { - assertThat(fqn.get(i), is(path.getSegment(i))); + assertThat((Path.Segment)fqn.get(i), is(path.getSegment(i))); } } @Test public void shouldCreateFullyQualifiedNodeOfPathSegmentsFromRootPath() { Path path = pathFactory.createRootPath(); - Fqn<Path.Segment> fqn = connection.getFullyQualifiedName(path); + Fqn<?> fqn = connection.getFullyQualifiedName(path); assertThat(fqn.size(), is(0)); assertThat(fqn.isRoot(), is(true)); } @@ -188,10 +199,10 @@ @Test public void shouldCreateFullyQualifiedNodeFromPathSegment() { Path.Segment segment = pathFactory.createSegment("a"); - Fqn<Path.Segment> fqn = connection.getFullyQualifiedName(segment); + Fqn<?> fqn = connection.getFullyQualifiedName(segment); assertThat(fqn.size(), is(1)); assertThat(fqn.isRoot(), is(false)); - assertThat(fqn.get(0), is(segment)); + assertThat((Path.Segment)fqn.get(0), is(segment)); } @Test( expected = AssertionError.class ) @@ -202,21 +213,20 @@ @Test public void shouldCreatePathFromFullyQualifiedNode() { Path path = pathFactory.create("/a/b/c/d"); - Fqn<Path.Segment> fqn = connection.getFullyQualifiedName(path); + Fqn<?> fqn = connection.getFullyQualifiedName(path); assertThat(connection.getPath(pathFactory, fqn), is(path)); } @Test public void shouldCreateRootPathFromRootFullyQualifiedNode() { Path path = pathFactory.createRootPath(); - Fqn<Path.Segment> fqn = connection.getFullyQualifiedName(path); + Fqn<?> fqn = connection.getFullyQualifiedName(path); assertThat(connection.getPath(pathFactory, fqn), is(path)); } @Test public void shouldGetNodeIfItExistsInCache() { // Set up the cache with data ... - stub(source.getUuidPropertyName()).toReturn(DnaLexicon.UUID.getString(context.getNamespaceRegistry())); Name uuidProperty = connection.getUuidPropertyName(context); Path[] paths = {pathFactory.create("/a"), pathFactory.create("/a/b"), pathFactory.create("/a/b/c")}; Path nonExistantPath = pathFactory.create("/a/d"); @@ -241,7 +251,6 @@ @Test public void shouldThrowExceptionWithLowestExistingNodeFromGetNodeIfTheNodeDoesNotExist() { // Set up the cache with data ... - stub(source.getUuidPropertyName()).toReturn(DnaLexicon.UUID.getString(context.getNamespaceRegistry())); Name uuidProperty = connection.getUuidPropertyName(context); Path[] paths = {pathFactory.create("/a"), pathFactory.create("/a/b"), pathFactory.create("/a/b/c")}; Path nonExistantPath = pathFactory.create("/a/d"); @@ -268,7 +277,6 @@ @Test public void shouldCopyNode() { // Set up the cache with data ... - stub(source.getUuidPropertyName()).toReturn(DnaLexicon.UUID.getString(context.getNamespaceRegistry())); Name uuidProperty = connection.getUuidPropertyName(context); Path[] paths = {pathFactory.create("/a"), pathFactory.create("/a/b"), pathFactory.create("/a/b/c"), pathFactory.create("/a/d")}; @@ -297,7 +305,7 @@ assertThat(newNodeB, is(nullValue())); assertThat(newNodeC, is(nullValue())); // Copy node B and place under node D - assertThat(connection.copyNode(nodeB, nodeD, true, uuidProperty), is(2)); + assertThat(connection.copyNode(nodeB, nodeD, true, uuidProperty, context), is(2)); newNodeB = cache.getNode(Fqn.fromList(newPathB.getSegmentsList())); newNodeC = cache.getNode(Fqn.fromList(newPathC.getSegmentsList())); assertThat(newNodeB, is(notNullValue())); @@ -307,4 +315,74 @@ assertThat(newNodeC.get(uuidProperty), is(not(nodeC.get(uuidProperty)))); } + @Test + public void shouldCreateSameNameSiblingsAndAutomaticallyManageSiblingIndexes() throws Exception { + // Set up the cache with some data, using different execute calls ... + Property prop1 = propertyFactory.create(nameFactory.create("dna:prop1"), "value1"); + Property prop2 = propertyFactory.create(nameFactory.create("dna:prop2"), "value1"); + Path pathA = pathFactory.create("/a"); + BasicCreateNodeCommand createNode = new BasicCreateNodeCommand(pathA); + createNode.setProperties(prop1, prop2); + connection.execute(context, createNode); + + for (int i = 0; i != 20; ++i) { + createNode = new BasicCreateNodeCommand(pathFactory.create("/a/b")); + createNode.setProperties(prop1, prop2); + connection.execute(context, createNode); + } + + // Get the name that we'll use in later assertions ... + Name nameB = pathFactory.createSegment("b").getName(); + assertThat(nameB.getLocalName(), is("b")); + + // Now verify the content ... + BasicGetNodeCommand getNode = new BasicGetNodeCommand(pathA); + connection.execute(context, getNode); + int index = 1; + for (Path.Segment segment : getNode.getChildren()) { + assertThat(segment.getName(), is(nameB)); + assertThat(segment.hasIndex(), is(true)); + assertThat(segment.getIndex(), is(index)); + ++index; + } + } + + @Test + public void shouldCreateSameNameSiblingsAndAutomaticallyManageSiblingIndexesInterspersedWithSiblingsWithOtherNames() + throws Exception { + // Set up the cache with some data, using different execute calls ... + Property prop1 = propertyFactory.create(nameFactory.create("dna:prop1"), "value1"); + Property prop2 = propertyFactory.create(nameFactory.create("dna:prop2"), "value1"); + Path pathA = pathFactory.create("/a"); + BasicCreateNodeCommand createNode = new BasicCreateNodeCommand(pathA); + createNode.setProperties(prop1, prop2); + connection.execute(context, createNode); + + for (int i = 0; i != 20; ++i) { + String path = i % 5 == 0 ? "/a/b" : "/a/c" + i; + createNode = new BasicCreateNodeCommand(pathFactory.create(path)); + createNode.setProperties(prop1, prop2); + connection.execute(context, createNode); + } + + // Get the name that we'll use in later assertions ... + Name nameB = pathFactory.createSegment("b").getName(); + assertThat(nameB.getLocalName(), is("b")); + + // Now verify the content ... + BasicGetNodeCommand getNode = new BasicGetNodeCommand(pathA); + connection.execute(context, getNode); + int index = 1; + for (Path.Segment segment : getNode.getChildren()) { + if (segment.getName().getLocalName().equals("b")) { + assertThat(segment.getName(), is(nameB)); + assertThat(segment.hasIndex(), is(true)); + assertThat(segment.getIndex(), is(index)); + ++index; + } else { + assertThat(segment.hasIndex(), is(false)); + } + } + } + } Modified: trunk/connectors/dna-connector-jbosscache/src/test/resources/log4j.properties =================================================================== --- trunk/connectors/dna-connector-jbosscache/src/test/resources/log4j.properties 2008-08-10 18:43:31 UTC (rev 413) +++ trunk/connectors/dna-connector-jbosscache/src/test/resources/log4j.properties 2008-08-11 20:30:45 UTC (rev 414) @@ -9,6 +9,7 @@ # Set up the default logging to be INFO level, then override specific units log4j.logger.org.jboss.dna=INFO +#log4j.logger.org.jboss.dna.connector.jbosscache=TRACE # JBoss Cache logging log4j.logger.org.jboss.cache=WARN, stdout Modified: trunk/dna-spi/src/main/java/org/jboss/dna/spi/graph/commands/executor/AbstractCommandExecutor.java =================================================================== --- trunk/dna-spi/src/main/java/org/jboss/dna/spi/graph/commands/executor/AbstractCommandExecutor.java 2008-08-10 18:43:31 UTC (rev 413) +++ trunk/dna-spi/src/main/java/org/jboss/dna/spi/graph/commands/executor/AbstractCommandExecutor.java 2008-08-11 20:30:45 UTC (rev 414) @@ -121,11 +121,9 @@ // The command could implement multiple "get" behaviors if (command instanceof GetNodeCommand) { execute((GetNodeCommand)command); - } - if (command instanceof GetPropertiesCommand) { + } else if (command instanceof GetPropertiesCommand) { execute((GetPropertiesCommand)command); - } - if (command instanceof GetChildrenCommand) { + } else if (command instanceof GetChildrenCommand) { execute((GetChildrenCommand)command); } // The command could record the branch even if deleting or moving ... Modified: trunk/dna-spi/src/main/java/org/jboss/dna/spi/graph/commands/impl/BasicCreateNodeCommand.java =================================================================== --- trunk/dna-spi/src/main/java/org/jboss/dna/spi/graph/commands/impl/BasicCreateNodeCommand.java 2008-08-10 18:43:31 UTC (rev 413) +++ trunk/dna-spi/src/main/java/org/jboss/dna/spi/graph/commands/impl/BasicCreateNodeCommand.java 2008-08-11 20:30:45 UTC (rev 414) @@ -22,6 +22,7 @@ package org.jboss.dna.spi.graph.commands.impl; import java.util.Collection; +import java.util.LinkedList; import net.jcip.annotations.NotThreadSafe; import org.jboss.dna.common.util.StringUtil; import org.jboss.dna.spi.graph.Path; @@ -44,6 +45,13 @@ /** * @param path the path to the node; may not be null + */ + public BasicCreateNodeCommand( Path path ) { + this(path, new LinkedList<Property>(), NodeConflictBehavior.UPDATE); + } + + /** + * @param path the path to the node; may not be null * @param properties the properties of the node; may not be null * @param conflictBehavior the desired behavior when a node exists at the <code>path</code>; may not be null */ @@ -92,6 +100,18 @@ this.properties.add(property); } + public void setProperties( Property... properties ) { + for (Property property : properties) { + setProperty(property); + } + } + + public void setProperties( Iterable<Property> properties ) { + for (Property property : properties) { + setProperty(property); + } + } + /** * {@inheritDoc} */ Modified: trunk/dna-spi/src/test/java/org/jboss/dna/spi/graph/commands/executor/AbstractCommandExecutorTest.java =================================================================== --- trunk/dna-spi/src/test/java/org/jboss/dna/spi/graph/commands/executor/AbstractCommandExecutorTest.java 2008-08-10 18:43:31 UTC (rev 413) +++ trunk/dna-spi/src/test/java/org/jboss/dna/spi/graph/commands/executor/AbstractCommandExecutorTest.java 2008-08-11 20:30:45 UTC (rev 414) @@ -121,8 +121,6 @@ verify(validator, times(1)).execute((GraphCommand)null); verify(validator, times(1)).execute((GraphCommand)createNodeCommand); verify(validator, times(2)).execute((GraphCommand)getNodeCommand); - verify(validator, times(2)).execute((GetPropertiesCommand)getNodeCommand); - verify(validator, times(2)).execute((GetChildrenCommand)getNodeCommand); verify(validator, times(1)).execute(createNodeCommand); verify(validator, times(2)).execute(getNodeCommand); verifyNoMoreInteractions(validator); @@ -138,8 +136,6 @@ verify(validator, times(1)).execute(command); verify(validator, times(1)).execute((GraphCommand)createNodeCommand); verify(validator, times(2)).execute((GraphCommand)getNodeCommand); - verify(validator, times(2)).execute((GetPropertiesCommand)getNodeCommand); - verify(validator, times(2)).execute((GetChildrenCommand)getNodeCommand); verify(validator, times(1)).execute(createNodeCommand); verify(validator, times(2)).execute(getNodeCommand); verifyNoMoreInteractions(validator); @@ -150,8 +146,8 @@ GetNodeCommand getNodeCommand = mock(GetNodeCommand.class); executor.execute((GraphCommand)getNodeCommand); verify(validator, times(1)).execute((GraphCommand)getNodeCommand); - verify(validator, times(1)).execute((GetPropertiesCommand)getNodeCommand); - verify(validator, times(1)).execute((GetChildrenCommand)getNodeCommand); + verify(validator, times(0)).execute((GetPropertiesCommand)getNodeCommand); + verify(validator, times(0)).execute((GetChildrenCommand)getNodeCommand); verify(validator, times(1)).execute(getNodeCommand); verifyNoMoreInteractions(validator); }