Index: dna-graph/pom.xml =================================================================== --- dna-graph/pom.xml (revision 854) +++ dna-graph/pom.xml (working copy) @@ -32,6 +32,11 @@ joda-time joda-time + + com.google.code.google-collections + google-collect + snapshot-20080530 + Index: dna-graph/src/main/java/org/jboss/dna/graph/io/Destination.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/io/Destination.java (revision 854) +++ dna-graph/src/main/java/org/jboss/dna/graph/io/Destination.java (working copy) @@ -66,6 +66,15 @@ Property... additionalProperties ); /** + * Sets the given properties on the node at the supplied path. The path will be absolute. + * + * @param path the absolute path of the node + * @param properties the remaining properties for the node + */ + public void setProperties( Path path, + Property... properties ); + + /** * Signal to this destination that any enqueued create requests should be submitted. Usually this happens at the end of the * document parsing, but an implementer must allow for it to be called multiple times and anytime during parsing. */ Index: dna-graph/src/main/java/org/jboss/dna/graph/io/GraphBatchDestination.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/io/GraphBatchDestination.java (revision 854) +++ dna-graph/src/main/java/org/jboss/dna/graph/io/GraphBatchDestination.java (working copy) @@ -117,6 +117,18 @@ /** * {@inheritDoc} + * + * @see org.jboss.dna.graph.io.Destination#setProperties(org.jboss.dna.graph.property.Path, org.jboss.dna.graph.property.Property[]) + */ + public void setProperties( Path path, + Property... properties ) { + if (properties == null) return; + + batch.set(properties).on(path); + } + + /** + * {@inheritDoc} * * @see org.jboss.dna.graph.io.Destination#submit() */ Index: dna-graph/src/main/java/org/jboss/dna/graph/request/BatchRequestBuilder.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/request/BatchRequestBuilder.java (revision 854) +++ dna-graph/src/main/java/org/jboss/dna/graph/request/BatchRequestBuilder.java (working copy) @@ -94,7 +94,7 @@ return newRequest; } // We have at least one other request, so add the pending request ... - add(newRequest); + addPending(); ++number; } else { // There is no pending request ... Index: dna-graph/src/main/java/org/jboss/dna/graph/xml/XmlHandler.java =================================================================== --- dna-graph/src/main/java/org/jboss/dna/graph/xml/XmlHandler.java (revision 854) +++ dna-graph/src/main/java/org/jboss/dna/graph/xml/XmlHandler.java (working copy) @@ -24,6 +24,8 @@ package org.jboss.dna.graph.xml; import java.util.ArrayList; +import java.util.Collection; +import java.util.Collections; import java.util.HashMap; import java.util.LinkedList; import java.util.List; @@ -45,6 +47,8 @@ import org.jboss.dna.graph.property.basic.LocalNamespaceRegistry; import org.xml.sax.Attributes; import org.xml.sax.ext.DefaultHandler2; +import com.google.common.collect.LinkedHashMultimap; +import com.google.common.collect.Multimap; /** * A {@link DefaultHandler2} specialization that responds to XML content events by creating the corresponding content in the @@ -145,6 +149,7 @@ */ protected Path currentPath; + /** * Flag the records whether the first element should be skipped. */ @@ -161,7 +166,20 @@ */ protected final Object[] propertyValues = new Object[1]; + + /** + * Character buffer to aggregate nested character data + * @see ElementEntry + */ + private StringBuilder characterDataBuffer = new StringBuilder(); + /** + * Stack of pending {@link ElementEntry element entries} from the root of the imported content to the current node. + * @see ElementEntry + */ + private final LinkedList elementStack = new LinkedList(); + + /** * Create a handler that creates content in the supplied graph * * @param destination the destination where the content should be sent.graph in which the content should be placed @@ -310,6 +328,16 @@ assert localName != null; Name nodeName = null; + ElementEntry element; + if (!elementStack.isEmpty()) { + // Add the parent + elementStack.peek().addAsNode(); + element = new ElementEntry(elementStack.peek(), currentPath, null); + } else { + element = new ElementEntry(null, currentPath, null); + } + elementStack.push(element); + properties.clear(); Object typePropertyValue = null; // Convert each of the attributes to a property ... @@ -333,6 +361,7 @@ // Check to see if this is an attribute that represents the node name (which may be null) ... if (nodeName == null && attributeName.equals(nameAttribute)) { nodeName = nameFactory.create(attributes.getValue(i)); // don't use a decoder + element.setName(nodeName); continue; } if (typePropertyValue == null && attributeName.equals(typeAttribute)) { @@ -340,13 +369,13 @@ continue; } // Create a property for this attribute ... - Property property = createProperty(attributeName, attributes.getValue(i)); - properties.add(property); + element.addProperty(attributeName, attributes.getValue(i)); } // Create the node name if required ... if (nodeName == null) { // No attribute defines the node name ... nodeName = nameFactory.create(uri, localName, decoder); + element.setName(nodeName); } else { typePropertyValue = nameFactory.create(uri, localName, decoder); } @@ -354,15 +383,12 @@ // A attribute defines the node name. Set the type property, if required if (typePropertyValue == null) typePropertyValue = typeAttributeValue; if (typePropertyValue != null) { - propertyValues[0] = typePropertyValue; - Property property = propertyFactory.create(typeAttribute, propertyValues); - properties.add(property); + element.addProperty(typeAttribute, typePropertyValue); } } + // Update the current path ... - currentPath = pathFactory.create(currentPath, nodeName); - // Create the node, and note that we don't care about same-name siblings (as the graph will correct them) ... - destination.create(currentPath, properties); + currentPath = element.path(); } /** @@ -374,6 +400,15 @@ public void endElement( String uri, String localName, String name ) { + + String s = characterDataBuffer.toString().trim(); + if (s.length() > 0) { + elementStack.pop().addAsPropertySetTo(s); + } else if (!elementStack.isEmpty()) { + elementStack.pop().submit(); + } + characterDataBuffer = new StringBuilder(); + // Nothing to do but to change the current path to be the parent ... currentPath = currentPath.getParent(); } @@ -381,6 +416,19 @@ /** * {@inheritDoc} * + * @see org.xml.sax.helpers.DefaultHandler#characters(char[], int, int) + */ + @Override + public void characters( char[] ch, + int start, + int length ) { + // Have to add this to a buffer as one logical set of character data can cause this method to fire multiple times + characterDataBuffer.append(ch, start, length); + } + + /** + * {@inheritDoc} + * * @see org.xml.sax.helpers.DefaultHandler#endDocument() */ @Override @@ -402,8 +450,113 @@ protected Property createProperty( Name propertyName, Object value ) { propertyValues[0] = value; - Property result = propertyFactory.create(propertyName, propertyValues); - return result; + return propertyFactory.create(propertyName, propertyValues); } + /** + * Create a property with the given name and values, obtained from an attribute name and value in the XML content. + *

+ * By default, this method creates a property by directly using the values as the values of the property. + *

+ * + * @param propertyName the name of the property; never null + * @param values the attribute values + * @return the property; may not be null + */ + protected Property createProperty( Name propertyName, + Collection values ) { + return propertyFactory.create(propertyName, values); + } + + /** + * Possible states for an {@link ElementEntry} instance. All element entries start in state {@code TBD} and then transition to + * one of the terminating states, {@code NODE} or {@code PROPERTY} when {@link ElementEntry#addAsNode()} or + * {@link ElementEntry#addAsPropertySetTo(Object)} is invoked. + */ + protected enum ElementEntryState { + NODE, + PROPERTY, + TBD + } + + /** + * Element entries hold references to the data of "pending" elements. "Pending" elements are elements which have been + * encountered through a {@link XmlHandler#startElement(String, String, String, Attributes)} event but have not yet been fully + * committed to the {@link XmlHandler#destination}. + *

+ * As the current import semantics allow elements with nested character data to be imported as properties, it is not always + * possible to determine whether the element represents a node or a property from within the {@code startElement} method. + * Therefore, {@code ElementEntries} are initially created in an {@link ElementEntryState#TBD unknown state} and submitted to + * the {@code destination} when it can be positively determined that the entry represents a property (if nested character data + * is encountered) or a node (if a child node is detected or the {@link XmlHandler#endElement(String, String, String)} method + * is invoked prior to encountering nested character data). + *

+ *

+ * As DNA does not currently support a way to add a value to an existing property through the Graph API, {@code + * ElementEntries} also contain a {@link Multimap} of property names to values. The node's properties are aggregated and only + * submitted to the {@code destination} when the {@link XmlHandler#endElement(String, String, String)} event fires. + *

+ */ + private class ElementEntry { + + private ElementEntry parent; + // Stored seperately since the root node has no parent ElementEntry but does have a path + private Path pathToParent; + private Path pathToThisNode; + private Name name; + private Multimap properties; + private ElementEntryState state; + + protected ElementEntry( ElementEntry parent, + Path pathToParent, + Name name ) { + this.parent = parent; + this.pathToParent = pathToParent; + this.name = name; + this.state = ElementEntryState.TBD; + properties = new LinkedHashMultimap(); + } + + protected void setName( Name name ) { + this.name = name; + pathToThisNode = pathFactory.create(pathToParent, name); + } + + protected void addProperty( Name propertyName, + Object propertyValue ) { + assert state != ElementEntryState.PROPERTY; + properties.put(propertyName, propertyValue); + } + + protected void addAsNode() { + assert state != ElementEntryState.PROPERTY; + if (state == ElementEntryState.NODE) return; + + state = ElementEntryState.NODE; + destination.create(pathFactory.create(pathToParent, name), Collections.emptyList()); + } + + protected void addAsPropertySetTo( Object value ) { + assert state != ElementEntryState.NODE; + state = ElementEntryState.PROPERTY; + parent.addProperty(name, value); + } + + protected final Path path() { + return pathToThisNode; + } + + protected void submit() { + if (state == ElementEntryState.PROPERTY) return; + if (state == ElementEntryState.TBD) addAsNode(); + + if (properties.size() == 0) return; + Property[] propertiesToAdd = new Property[properties.size()]; + int i = 0; + for (Name name : properties.keySet()) { + propertiesToAdd[i++] = createProperty(name, properties.get(name)); + } + destination.setProperties(pathToThisNode, propertiesToAdd); + } + } } Index: dna-graph/src/test/java/org/jboss/dna/graph/io/GraphImporterTest.java =================================================================== --- dna-graph/src/test/java/org/jboss/dna/graph/io/GraphImporterTest.java (revision 854) +++ dna-graph/src/test/java/org/jboss/dna/graph/io/GraphImporterTest.java (working copy) @@ -26,6 +26,7 @@ import static org.hamcrest.core.Is.is; import static org.hamcrest.core.IsInstanceOf.instanceOf; import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; import static org.mockito.Mockito.stub; import java.io.File; import java.net.URI; @@ -51,6 +52,8 @@ import org.jboss.dna.graph.request.CompositeRequest; import org.jboss.dna.graph.request.CreateNodeRequest; import org.jboss.dna.graph.request.Request; +import org.jboss.dna.graph.request.SetPropertyRequest; +import org.jboss.dna.graph.request.UpdatePropertiesRequest; import org.jboss.dna.graph.request.VerifyWorkspaceRequest; import org.junit.Before; import org.junit.Test; @@ -98,8 +101,8 @@ assertThat(lastExecutedRequest, is(instanceOf(CompositeRequest.class))); Iterator iter = ((CompositeRequest)lastExecutedRequest).iterator(); // assertCreateNode(iter, "/a/b/", "jcr:primaryType={http://www.jboss.org/dna/xml/1.0}document"); - assertCreateNode(iter, "/a/b/dna:system[1]", "jcr:primaryType={http://www.jcp.org/jcr/nt/1.0}unstructured"); - assertCreateNode(iter, "/a/b/dna:system[1]/dna:sources[1]", "jcr:primaryType={http://www.jcp.org/jcr/nt/1.0}unstructured"); + assertCreateNode(iter, "/a/b/dna:system[1]"); + assertCreateNode(iter, "/a/b/dna:system[1]/dna:sources[1]"); assertCreateNode(iter, "/a/b/dna:system[1]/dna:sources[1]/sourceA[1]", "repositoryName=repositoryA", @@ -111,6 +114,8 @@ "repositoryName=repositoryB", "jcr:primaryType={http://www.jcp.org/jcr/nt/1.0}unstructured", "dna:classname=org.jboss.dna.connector.inmemory.InMemoryRepositorySource"); + assertCreateProperties(iter, "/a/b/dna:system[1]/dna:sources[1]", "jcr:primaryType={http://www.jcp.org/jcr/nt/1.0}unstructured"); + assertCreateProperties(iter, "/a/b/dna:system[1]", "jcr:primaryType={http://www.jcp.org/jcr/nt/1.0}unstructured"); assertThat(iter.hasNext(), is(false)); } @@ -124,8 +129,37 @@ Path parentPath = createNode.under().getPath(); assertThat(parentPath, is(expectedPath.getParent())); assertThat(createNode.named(), is(expectedPath.getLastSegment().getName())); + + if (properties.length > 0) { + assertCreateProperties(iterator, path, properties); + } + } + + public void assertCreateProperties( Iterator iterator, + String path, + String... properties ) { + Request nextCommand = iterator.next(); + + if (nextCommand instanceof UpdatePropertiesRequest) { + assertUpdateProperties((UpdatePropertiesRequest) nextCommand, path, properties); + } + else if (nextCommand instanceof SetPropertyRequest) { + assertSetProperty((SetPropertyRequest) nextCommand, path, properties); + } + else { + fail("Invalid next request type: " + nextCommand.getClass().getName()); + } + + } + public void assertUpdateProperties( UpdatePropertiesRequest createNode, + String path, + String... properties ) { + Path expectedPath = context.getValueFactories().getPathFactory().create(path); + Path parentPath = createNode.changedLocation().getPath().getParent(); + assertThat(parentPath, is(expectedPath.getParent())); + assertThat(createNode.changedLocation().getPath().getLastSegment().getName(), is(expectedPath.getLastSegment().getName())); Map propertiesByName = new HashMap(); - for (Property prop : createNode.properties()) { + for (Property prop : createNode.properties().values()) { propertiesByName.put(prop.getName(), prop); } for (String propertyStr : properties) { @@ -150,6 +184,40 @@ assertThat(propertiesByName.isEmpty(), is(true)); } + public void assertSetProperty( SetPropertyRequest createNode, + String path, + String... properties ) { + Path expectedPath = context.getValueFactories().getPathFactory().create(path); + Path parentPath = createNode.changedLocation().getPath().getParent(); + assertThat(parentPath, is(expectedPath.getParent())); + assertThat(createNode.changedLocation().getPath().getLastSegment().getName(), is(expectedPath.getLastSegment().getName())); + Map propertiesByName = new HashMap(); + Property prop = createNode.property(); + propertiesByName.put(prop.getName(), prop); + + for (String propertyStr : properties) { + if (propertyStr == "any properties") { + propertiesByName.clear(); + break; + } + Matcher matcher = Pattern.compile("([^=]+)=(.*)").matcher(propertyStr); + if (!matcher.matches()) continue; + System.out.println("Property: " + propertyStr + " ==> " + matcher); + Name propertyName = context.getValueFactories().getNameFactory().create(matcher.group(1)); + System.out.println("Property name: " + matcher.group(1)); + String value = matcher.group(2); // doesn't handle multiple values!! + if (value.trim().length() == 0) value = null; + Property actual = propertiesByName.remove(propertyName); + Property expectedProperty = context.getPropertyFactory().create(propertyName, value); + assertThat("missing property " + propertyName, actual, is(expectedProperty)); + } + if (!propertiesByName.isEmpty()) { + System.out.println("Properties for " + path + "\n" + propertiesByName); + } + assertThat(propertiesByName.isEmpty(), is(true)); + } + + protected class MockRepositoryConnection implements RepositoryConnection { public void close() { } Index: dna-graph/src/test/java/org/jboss/dna/graph/xml/XmlHandlerTest.java =================================================================== --- dna-graph/src/test/java/org/jboss/dna/graph/xml/XmlHandlerTest.java (revision 854) +++ dna-graph/src/test/java/org/jboss/dna/graph/xml/XmlHandlerTest.java (working copy) @@ -350,14 +350,18 @@ parse("xmlHandler/docWithoutNamespaces.xml"); // Check the generated content; note that the attribute name doesn't match, so the nodes don't get special names String unstructPrimaryType = "jcr:primaryType={http://www.jcp.org/jcr/nt/1.0}unstructured"; - assertNode("Cars", unstructPrimaryType); - assertNode("Cars/Hybrid", unstructPrimaryType); + assertNode("Cars"); + assertNode("Cars/Hybrid"); assertNode("Cars/Hybrid/car", unstructPrimaryType, "name=Toyota Prius", "maker=Toyota", "model=Prius"); assertNode("Cars/Hybrid/car", unstructPrimaryType, "name=Toyota Highlander", "maker=Toyota", "model=Highlander"); assertNode("Cars/Hybrid/car", unstructPrimaryType, "name=Nissan Altima", "maker=Nissan", "model=Altima"); - assertNode("Cars/Sports", unstructPrimaryType); + assertProperties("Cars/Hybrid", unstructPrimaryType); + assertNode("Cars/Sports"); assertNode("Cars/Sports/car", unstructPrimaryType, "name=Aston Martin DB9", "maker=Aston Martin", "model=DB9"); assertNode("Cars/Sports/car", unstructPrimaryType, "name=Infiniti G37", "maker=Infiniti", "model=G37"); + assertProperties("Cars/Sports", unstructPrimaryType); + assertProperties("Cars", unstructPrimaryType); + } @Test @@ -371,21 +375,63 @@ // Check the generated content; note that the attribute name doesn't match, so the nodes don't get special names String unstructPrimaryType = "jcr:primaryType={http://www.jcp.org/jcr/nt/1.0}unstructured"; String carPrimaryType = "jcr:primaryType={http://default.namespace.com}car"; - assertNode("c:Cars", unstructPrimaryType); - assertNode("c:Cars/c:Hybrid", unstructPrimaryType); + assertNode("c:Cars"); + assertNode("c:Cars/c:Hybrid"); assertNode("c:Cars/c:Hybrid/c:Toyota Prius", carPrimaryType, "c:maker=Toyota", "c:model=Prius"); assertNode("c:Cars/c:Hybrid/c:Toyota Highlander", carPrimaryType, "c:maker=Toyota", "c:model=Highlander"); assertNode("c:Cars/c:Hybrid/c:Nissan Altima", carPrimaryType, "c:maker=Nissan", "c:model=Altima"); - assertNode("c:Cars/c:Sports", unstructPrimaryType); + assertProperties("c:Cars/c:Hybrid", unstructPrimaryType); + assertNode("c:Cars/c:Sports"); assertNode("c:Cars/c:Sports/c:Aston Martin DB9", carPrimaryType, "c:maker=Aston Martin", "c:model=DB9"); assertNode("c:Cars/c:Sports/c:Infiniti G37", carPrimaryType, "c:maker=Infiniti", "c:model=G37"); + assertProperties("c:Cars/c:Sports", unstructPrimaryType); + assertProperties("c:Cars", unstructPrimaryType); } + @Test + public void shouldParseXmlDocumentWithNestedPropertiesShouldPlaceContentUnderRootNode() throws IOException, SAXException { + handler = new XmlHandler(destination, skipRootElement, parentPath, decoder, nameAttribute, typeAttribute, + typeAttributeValue, scoping); + parse("xmlHandler/docWithNestedProperties.xml"); + // Check the generated content; note that the attribute name DOES match, so the nodes names come from "jcr:name" attribute + assertNode("Cars"); + assertNode("Cars/Hybrid"); + assertNode("Cars/Hybrid/car", "name=Toyota Prius", "maker=Toyota", "model=Prius"); + assertNode("Cars/Hybrid/car", "name=Toyota Highlander", "maker=Toyota", "model=Highlander"); + assertNode("Cars/Hybrid/car", "name=Nissan Altima", "maker=Nissan", "model=Altima"); + assertNode("Cars/Sports"); + assertNode("Cars/Sports/car", "name=Aston Martin DB9", "maker=Aston Martin", "model=DB9"); + assertNode("Cars/Sports/car"); + assertNode("Cars/Sports/car/driver", "name=Tony Stewart"); + assertProperties("Cars/Sports/car", "name=Infiniti G37", "maker=Infiniti", "model=G37", "category=Turbocharged=My Sedan"); + } + protected void assertNode( String path, String... properties ) { // Create the expected path ... PathFactory factory = context.getValueFactories().getPathFactory(); Path expectedPath = parentPath != null ? factory.create(parentPath, path) : factory.create("/" + path); + // Now get the next request and compare the expected and actual ... + CreateNodeRequest request = requests.remove(); + Path parentPath = request.under().getPath(); + assertThat(parentPath, is(expectedPath.getParent())); + assertThat(request.named(), is(expectedPath.getLastSegment().getName())); + + if (properties.length == 0) { + if (!requests.isEmpty()) { + assertThat(requests.peek().properties().size(), is(0)); + } + } + else { + assertProperties(path, properties); + } + } + + protected void assertProperties( String path, + String... properties ) { + // Create the expected path ... + PathFactory factory = context.getValueFactories().getPathFactory(); + Path expectedPath = parentPath != null ? factory.create(parentPath, path) : factory.create("/" + path); // Create the list of properties ... Map expectedProperties = new HashMap(); for (String propertyString : properties) { @@ -399,12 +445,13 @@ Property property = context.getPropertyFactory().create(name, values); expectedProperties.put(name, property); } - // Now get the next request and compare the expected and actual ... - CreateNodeRequest request = requests.remove(); - Path parentPath = request.under().getPath(); + + CreateNodeRequest propertyRequest = requests.remove(); + Path parentPath = propertyRequest.under().getPath(); assertThat(parentPath, is(expectedPath.getParent())); - assertThat(request.named(), is(expectedPath.getLastSegment().getName())); - for (Property actual : request.properties()) { + assertThat(propertyRequest.named(), is(expectedPath.getLastSegment().getName())); + + for (Property actual : propertyRequest.properties()) { Property expected = expectedProperties.remove(actual.getName()); assertThat("unexpected property: " + actual, expected, is(notNullValue())); assertThat(actual, is(expected)); @@ -417,6 +464,8 @@ else isFirst = false; msg.append(expected.getName()); } + msg.append(" on node ").append(propertyRequest.under()); + System.out.println("Found properties: " + propertyRequest.properties()); assertThat(msg.toString(), expectedProperties.isEmpty(), is(true)); } } @@ -481,6 +530,21 @@ } } + public void setProperties( Path path, + Property... properties ) { + if (properties.length == 0) { + return; + } + else if (properties.length == 1) { + create(path, properties[0]); + } + else { + Property[] additionalProperties = new Property[properties.length - 1]; + System.arraycopy(properties, 1, additionalProperties, 0, properties.length - 1); + create(path, properties[0], additionalProperties); + } + } + @SuppressWarnings( "synthetic-access" ) public ExecutionContext getExecutionContext() { return XmlHandlerTest.this.context; Index: dna-graph/src/test/resources/xmlHandler/docWithNestedProperties.xml =================================================================== --- dna-graph/src/test/resources/xmlHandler/docWithNestedProperties.xml (revision 0) +++ dna-graph/src/test/resources/xmlHandler/docWithNestedProperties.xml (revision 0) @@ -0,0 +1,22 @@ + + + + + + + Nissan + Altima + + + + + + Infiniti + Turbocharged + + G37 + My + + + +