Author: bcarothers
Date: 2009-06-10 10:53:55 -0400 (Wed, 10 Jun 2009)
New Revision: 1028
Modified:
trunk/dna-repository/src/main/java/org/jboss/dna/repository/sequencer/StreamSequencerAdapter.java
trunk/dna-repository/src/test/java/org/jboss/dna/repository/sequencer/StreamSequencerAdapterTest.java
Log:
DNA-443 Java sequencer is not saving content correctly
Applied patch that changes all of the node creation to use the destination instead of
doing direct writes to the graph. Had to add an accumulator to keep track of the created
paths. This should have a beneficial effect on performance over the use of
createIfMissing, since it avoids having to continually read in the nodes at the root of
the graph. The patch also adds a new test case to verify that the StreamSequencerAdapter
is not creating extra nodes.
Modified:
trunk/dna-repository/src/main/java/org/jboss/dna/repository/sequencer/StreamSequencerAdapter.java
===================================================================
---
trunk/dna-repository/src/main/java/org/jboss/dna/repository/sequencer/StreamSequencerAdapter.java 2009-06-10
04:43:17 UTC (rev 1027)
+++
trunk/dna-repository/src/main/java/org/jboss/dna/repository/sequencer/StreamSequencerAdapter.java 2009-06-10
14:53:55 UTC (rev 1028)
@@ -138,6 +138,9 @@
}
}
+ // Accumulator of paths that we've added to the batch but have not yet been
submitted to the graph
+ Set<Path> builtPaths = new HashSet<Path>();
+
// Find each output node and save the image metadata there ...
for (RepositoryNodePath outputPath : outputPaths) {
// Get the name of the repository workspace and the path to the output node
@@ -147,11 +150,11 @@
// Find or create the output node in this session ...
context.graph().useWorkspace(repositoryWorkspaceName);
- buildPathTo(nodePath, context);
- Node outputNode = context.graph().getNodeAt(nodePath);
+ buildPathTo(nodePath, context, builtPaths);
+ // Node outputNode = context.graph().getNodeAt(nodePath);
// Now save the image metadata to the output node ...
- saveOutput(outputNode, output, context);
+ saveOutput(nodePath, output, context, builtPaths);
}
context.getDestination().submit();
@@ -162,13 +165,15 @@
*
* @param nodePath the node path to create
* @param context the sequencer context under which it should be created
+ * @param builtPaths a set of the paths that have already been created but not
submitted in this batch
*/
private void buildPathTo( String nodePath,
- SequencerContext context ) {
+ SequencerContext context,
+ Set<Path> builtPaths ) {
PathFactory pathFactory =
context.getExecutionContext().getValueFactories().getPathFactory();
Path targetPath = pathFactory.create(nodePath);
- buildPathTo(targetPath, context);
+ buildPathTo(targetPath, context, builtPaths);
}
/**
@@ -176,9 +181,11 @@
*
* @param targetPath the node path to create
* @param context the sequencer context under which it should be created
+ * @param builtPaths a set of the paths that have already been created but not
submitted in this batch
*/
private void buildPathTo( Path targetPath,
- SequencerContext context ) {
+ SequencerContext context,
+ Set<Path> builtPaths ) {
PathFactory pathFactory =
context.getExecutionContext().getValueFactories().getPathFactory();
PropertyFactory propFactory =
context.getExecutionContext().getPropertyFactory();
@@ -186,20 +193,19 @@
Path workingPath = pathFactory.createRootPath();
Path.Segment[] segments = targetPath.getSegmentsArray();
int i = 0;
- if (segments.length > 1) {
- Property primaryType = propFactory.create(JcrLexicon.PRIMARY_TYPE,
JcrNtLexicon.UNSTRUCTURED);
- for (int max = segments.length - 1; i < max; i++) {
- workingPath = pathFactory.create(workingPath, segments[i]);
-
+ Property primaryType = propFactory.create(JcrLexicon.PRIMARY_TYPE,
JcrNtLexicon.UNSTRUCTURED);
+ for (int max = segments.length; i < max; i++) {
+ workingPath = pathFactory.create(workingPath, segments[i]);
+
+ if (!builtPaths.contains(workingPath)) {
try {
context.graph().getNodeAt(workingPath);
+ } catch (PathNotFoundException pnfe) {
+ context.getDestination().create(workingPath, primaryType);
+ builtPaths.add(workingPath);
}
- catch (PathNotFoundException pnfe) {
- context.graph().create(workingPath, primaryType);
- }
}
}
- context.graph().create(targetPath);
}
/**
@@ -209,15 +215,16 @@
* @param outputNode the existing node onto (or below) which the output is to be
written; never null
* @param output the (immutable) sequencing output; never null
* @param context the execution context for this sequencing operation; never null
+ * @param builtPaths a set of the paths that have already been created but not
submitted in this batch
*/
- protected void saveOutput( Node outputNode,
+ protected void saveOutput( String nodePath,
SequencerOutputMap output,
- SequencerContext context ) {
+ SequencerContext context,
+ Set<Path> builtPaths ) {
if (output.isEmpty()) return;
final PathFactory pathFactory =
context.getExecutionContext().getValueFactories().getPathFactory();
final PropertyFactory propertyFactory =
context.getExecutionContext().getPropertyFactory();
- // TODO: Is it safe to assume that the location for this node will include a
path?
- final Path outputNodePath =
pathFactory.create(outputNode.getLocation().getPath());
+ final Path outputNodePath = pathFactory.create(nodePath);
// Iterate over the entries in the output, in Path's natural order (shorter
paths first and in lexicographical order by
// prefix and name)
@@ -230,15 +237,15 @@
List<Property> properties = new LinkedList<Property>();
// Set all of the properties on this
for (SequencerOutputMap.PropertyValue property : entry.getPropertyValues())
{
- // String propertyName = property.getName().getString(namespaceRegistry,
Path.NO_OP_ENCODER);
properties.add(propertyFactory.create(property.getName(),
property.getValue()));
// TODO: Handle reference properties - currently passed in as Paths
}
if (absolutePath.getParent() != null) {
- buildPathTo(absolutePath.getParent(), context);
+ buildPathTo(absolutePath.getParent(), context, builtPaths);
}
- context.graph().create(absolutePath, properties);
+ context.getDestination().create(absolutePath, properties);
+ builtPaths.add(absolutePath);
}
}
@@ -260,8 +267,7 @@
Path path = factories.getPathFactory().create(input.getLocation().getPath());
Set<org.jboss.dna.graph.property.Property> props = new
HashSet<org.jboss.dna.graph.property.Property>(
-
input.getPropertiesByName()
-
.values());
+
input.getPropertiesByName().values());
props = Collections.unmodifiableSet(props);
String mimeType = getMimeType(context, sequencedProperty,
path.getLastSegment().getName().getLocalName());
return new StreamSequencerContext(context.getExecutionContext(), path, props,
mimeType, problems);
Modified:
trunk/dna-repository/src/test/java/org/jboss/dna/repository/sequencer/StreamSequencerAdapterTest.java
===================================================================
---
trunk/dna-repository/src/test/java/org/jboss/dna/repository/sequencer/StreamSequencerAdapterTest.java 2009-06-10
04:43:17 UTC (rev 1027)
+++
trunk/dna-repository/src/test/java/org/jboss/dna/repository/sequencer/StreamSequencerAdapterTest.java 2009-06-10
14:53:55 UTC (rev 1028)
@@ -37,6 +37,8 @@
import java.util.EnumSet;
import java.util.HashSet;
import java.util.Iterator;
+import java.util.List;
+import java.util.Map;
import java.util.Set;
import org.jboss.dna.common.collection.Problems;
import org.jboss.dna.common.collection.SimpleProblems;
@@ -48,6 +50,7 @@
import org.jboss.dna.graph.mimetype.MimeTypeDetectors;
import org.jboss.dna.graph.observe.NetChangeObserver.ChangeType;
import org.jboss.dna.graph.observe.NetChangeObserver.NetChange;
+import org.jboss.dna.graph.property.Name;
import org.jboss.dna.graph.property.Path;
import org.jboss.dna.graph.property.PathNotFoundException;
import org.jboss.dna.graph.property.Property;
@@ -527,7 +530,61 @@
problems);
assertThat(sequencerContext.getMimeType(), is("text/plain"));
}
+
+ private Name nameFor(String raw) {
+ return context.getValueFactories().getNameFactory().create(raw);
+ }
+
+ @Test
+ public void shouldNotCreateExtraNodesWhenSavingOutput() throws Exception {
+ SequencerOutputMap output = new SequencerOutputMap(context.getValueFactories());
+ Map<Name, Property> props;
+
+ /*
+ * Create several output properties and make sure the resulting graph
+ * does not contain duplicate nodes
+ */
+ output.setProperty("a", "property1", "value1");
+ output.setProperty("a/b", "property1", "value1");
+ output.setProperty("a/b", "property2", "value2");
+ output.setProperty("a/b[2]", "property1",
"value1");
+ output.setProperty("a/b[2]/c", "property1",
"value1");
+
+ Set<Path> builtPaths = new HashSet<Path>();
+ sequencer.saveOutput("/", output, seqContext, builtPaths);
+ seqContext.getDestination().submit();
+
+ Node rootNode = graph.getNodeAt("/");
+ assertThat(rootNode.getChildren().size(), is(1));
+
+ Node nodeA = graph.getNodeAt("/a");
+ props = nodeA.getPropertiesByName();
+ assertThat(nodeA.getChildren().size(), is(2));
+ assertThat(props.size(), is(2)); // Need to add one to account for dna:uuid
+ assertThat(props.get(nameFor("property1")).getFirstValue().toString(),
is("value1"));
+
+ Node nodeB = graph.getNodeAt("/a/b[1]");
+ props = nodeB.getPropertiesByName();
+
+ assertThat(props.size(), is(3)); // Need to add one to account for dna:uuid
+ assertThat(props.get(nameFor("property1")).getFirstValue().toString(),
is("value1"));
+ assertThat(props.get(nameFor("property2")).getFirstValue().toString(),
is("value2"));
+
+ Node nodeB2 = graph.getNodeAt("/a/b[2]");
+ props = nodeB2.getPropertiesByName();
+
+ assertThat(props.size(), is(2)); // Need to add one to account for dna:uuid
+ assertThat(props.get(nameFor("property1")).getFirstValue().toString(),
is("value1"));
+
+ Node nodeC = graph.getNodeAt("/a/b[2]/c");
+ props = nodeC.getPropertiesByName();
+
+ assertThat(props.size(), is(2)); // Need to add one to account for dna:uuid
+ assertThat(props.get(nameFor("property1")).getFirstValue().toString(),
is("value1"));
+
+ }
+
private void verifyProperty( StreamSequencerContext context,
String name,
Object... values ) {