Author: bcarothers Date: 2009-07-26 12:33:57 -0400 (Sun, 26 Jul 2009) New Revision: 1136 Added: trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/UpdateValuesRequest.java Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/Graph.java trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/RequestBuilder.java trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/processor/RequestProcessor.java trunk/dna-graph/src/test/java/org/jboss/dna/graph/connector/test/WritableConnectorTest.java Log: DNA-397 Graph API should allow adding and removing property values Committed patch that implements adding and removing values using addValue(value).andValue(value).to(propertyName).on(location) and removeValue(value).removeValue(value).from(propertyName).on(location) using a new request (UpdateValuesRequest) that has a reasonably efficient default implementation on RequestProcessor. The operations had to be named addValue/removeValue instead of add/remove to disambiguate the operations from the existing remove(String... propertyNames) grammar. The patch also includes a few new tests in WritableConnectorTest to validate the behavior. Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/Graph.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/Graph.java 2009-07-26 04:10:00 UTC (rev 1135) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/Graph.java 2009-07-26 16:33:57 UTC (rev 1136) @@ -57,6 +57,7 @@ import org.jboss.dna.graph.property.Name; import org.jboss.dna.graph.property.NameFactory; import org.jboss.dna.graph.property.Path; +import org.jboss.dna.graph.property.PathFactory; import org.jboss.dna.graph.property.PathNotFoundException; import org.jboss.dna.graph.property.Property; import org.jboss.dna.graph.property.PropertyFactory; @@ -1218,6 +1219,34 @@ }; } + public AddValue<Graph> addValue( Object value ) { + return new AddValueAction<Graph>(this, this.getCurrentWorkspaceName(), value) { + + @Override + protected Graph submit( String workspaceName, + Location on, + Name property, + List<Object> values ) { + requests.addValues(workspaceName, on, property, values); + return nextGraph.and(); + } + }; + } + + public RemoveValue<Graph> removeValue( Object value ) { + return new RemoveValueAction<Graph>(this, this.getCurrentWorkspaceName(), value) { + + @Override + protected Graph submit( String workspaceName, + Location on, + Name property, + List<Object> values ) { + requests.removeValues(workspaceName, on, property, values); + return nextGraph.and(); + } + }; + } + /** * Set the properties on a node. * @@ -3002,6 +3031,34 @@ }; } + public AddValue<Batch> addValue( Object value ) { + return new AddValueAction<Batch>(this, this.getCurrentWorkspaceName(), value) { + + @Override + protected Batch submit( String workspaceName, + Location on, + Name property, + List<Object> values ) { + requests.addValues(workspaceName, on, property, values); + return nextRequests.and(); + } + }; + } + + public RemoveValue<Batch> removeValue( Object value ) { + return new RemoveValueAction<Batch>(this, this.getCurrentWorkspaceName(), value) { + + @Override + protected Batch submit( String workspaceName, + Location on, + Name property, + List<Object> values ) { + requests.removeValues(workspaceName, on, property, values); + return nextRequests.and(); + } + }; + } + /** * Set the properties on a node. * @@ -4080,6 +4137,32 @@ } /** + * A component that defines the name of a property to which a value should be added. + * + * @param <Next> The interface that is to be returned when this request is completed + */ + public interface ToName<Next> { + + public Next to( String name ); + + public Next to( Name name ); + + } + + /** + * A component that defines the name of a property from which a value should be removed. + * + * @param <Next> The interface that is to be returned when this request is completed + */ + public interface FromName<Next> { + + public Next from( String name ); + + public Next from( Name name ); + + } + + /** * A component that defines the location to which a node should be copied or moved. * * @param <Next> The interface that is to be returned when this request is completed @@ -4945,6 +5028,189 @@ } /** + * The interface for defining the node on which an {@link Graph#addValue(Object)} operation applies and what additional values + * (if any) should be added. + * + * @param <Next> The interface that is to be returned when the request is completed + */ + public interface AddValue<Next> extends ToName<On<Next>> { + /** + * Specifies an additional value to be added + * + * @param value the value to be added + * @return an object that allows additional values to be specified for removal or for their location to be specified + */ + AddValue<Next> andValue( Object value ); + + } + + /** + * The interface for defining the node on which an {@link Graph#removeValue(Object)} operation applies and what additional + * values (if any) should be removed. + * + * @param <Next> The interface that is to be returned when the request is completed + */ + public interface RemoveValue<Next> extends FromName<On<Next>> { + + /** + * Specifies an additional value to be removed + * + * @param value the value to be removed + * @return an object that allows additional values to be specified for removal or for their location to be specified + */ + RemoveValue<Next> andValue( Object value ); + + } + + public abstract class AddValueAction<T> extends AbstractAction<T> implements AddValue<T> { + + private final String workspaceName; + private final List<Object> values = new LinkedList<Object>(); + + protected AddValueAction( T afterConjunction, + String workspaceName, + Object firstValue ) { + super(afterConjunction); + + this.workspaceName = workspaceName; + this.values.add(firstValue); + } + + public AddValue<T> andValue( Object nextValue ) { + this.values.add(nextValue); + return this; + } + + public On<T> to( String name ) { + NameFactory nameFactory = context.getValueFactories().getNameFactory(); + + return to(nameFactory.create(name)); + } + + public On<T> to( final Name name ) { + return new On<T>() { + + @Override + public T on( Iterable<Property> idProperties ) { + return on(Location.create(idProperties)); + } + + @Override + public T on( Location to ) { + return submit(workspaceName, to, name, values); + } + + @Override + public T on( Path to ) { + return on(Location.create(to)); + } + + @Override + public T on( Property firstIdProperty, + Property... additionalIdProperties ) { + return on(Location.create(firstIdProperty, additionalIdProperties)); + } + + @Override + public T on( Property idProperty ) { + return on(Location.create(idProperty)); + } + + @Override + public T on( String toPath ) { + PathFactory pathFactory = context.getValueFactories().getPathFactory(); + return on(Location.create(pathFactory.create(toPath))); + } + + @Override + public T on( UUID to ) { + return on(Location.create(to)); + } + }; + } + + protected abstract T submit( String workspaceName, + Location on, + Name property, + List<Object> values ); + + } + + public abstract class RemoveValueAction<T> extends AbstractAction<T> implements RemoveValue<T> { + + private final String workspaceName; + private final List<Object> values = new LinkedList<Object>(); + + protected RemoveValueAction( T afterConjunction, + String workspaceName, + Object firstValue ) { + super(afterConjunction); + + this.workspaceName = workspaceName; + this.values.add(firstValue); + } + + public RemoveValue<T> andValue( Object nextValue ) { + this.values.add(nextValue); + return this; + } + + public On<T> from( String name ) { + NameFactory nameFactory = context.getValueFactories().getNameFactory(); + + return from(nameFactory.create(name)); + } + + public On<T> from( final Name name ) { + return new On<T>() { + + @Override + public T on( Iterable<Property> idProperties ) { + return on(Location.create(idProperties)); + } + + @Override + public T on( Location to ) { + return submit(workspaceName, to, name, values); + } + + @Override + public T on( Path to ) { + return on(Location.create(to)); + } + + @Override + public T on( Property firstIdProperty, + Property... additionalIdProperties ) { + return on(Location.create(firstIdProperty, additionalIdProperties)); + } + + @Override + public T on( Property idProperty ) { + return on(Location.create(idProperty)); + } + + @Override + public T on( String toPath ) { + PathFactory pathFactory = context.getValueFactories().getPathFactory(); + return on(Location.create(pathFactory.create(toPath))); + } + + @Override + public T on( UUID to ) { + return on(Location.create(to)); + } + }; + } + + protected abstract T submit( String workspaceName, + Location on, + Name property, + List<Object> values ); + + } + + /** * A component used to set the values on a property. * * @param <Next> the next command Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/RequestBuilder.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/RequestBuilder.java 2009-07-26 04:10:00 UTC (rev 1135) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/RequestBuilder.java 2009-07-26 16:33:57 UTC (rev 1136) @@ -25,6 +25,7 @@ import java.util.HashMap; import java.util.Iterator; +import java.util.List; import java.util.Map; import org.jboss.dna.graph.Location; import org.jboss.dna.graph.NodeConflictBehavior; @@ -578,4 +579,41 @@ String workspaceName ) { return process(new DeleteBranchRequest(at, workspaceName)); } + + /** + * Add a request to add values to a property on an existing node + * + * @param workspaceName the name of the workspace containing the node; may not be null + * @param on the location of the node; may not be null + * @param property the name of the property; may not be null + * @param values the new values to add; may not be null + * @return the request; never null + */ + public UpdateValuesRequest addValues( String workspaceName, + Location on, + Name property, + List<Object> values ) { + UpdateValuesRequest request = new UpdateValuesRequest(workspaceName, on, property, values, null); + process(request); + return request; + } + + /** + * Add a request to remove values from a property on an existing node + * + * @param workspaceName the name of the workspace containing the node; may not be null + * @param on the location of the node; may not be null + * @param property the name of the property; may not be null + * @param values the new values to remove; may not be null + * @return the request; never null + */ + public UpdateValuesRequest removeValues( String workspaceName, + Location on, + Name property, + List<Object> values ) { + UpdateValuesRequest request = new UpdateValuesRequest(workspaceName, on, property, null, values); + process(request); + return request; + } + } Added: trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/UpdateValuesRequest.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/UpdateValuesRequest.java (rev 0) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/UpdateValuesRequest.java 2009-07-26 16:33:57 UTC (rev 1136) @@ -0,0 +1,176 @@ +package org.jboss.dna.graph.request; + +import java.util.Collections; +import java.util.List; +import org.jboss.dna.graph.GraphI18n; +import org.jboss.dna.graph.Location; +import org.jboss.dna.graph.property.Name; +import org.jboss.dna.graph.property.Path; +import org.jboss.dna.graph.property.Property; + +/** + * Instruction to update the values for a certain property on the node at the specified location. + * <p> + * This request is capable of specifying specific values for the property that will be added or removed. Other values for the + * property not be affected by this request. The request contains a workspace name and a location that uniquely identify a node in + * the workspace as well as the name of property (that may or may not previously exist) on the node. The request also contains + * zero or more values to add and zero or more values to remove from the property. All values will be appended to the list of + * values. Removals are processed before additions. + * </p> + * <p> + * Even if the property has no values after this call, the property itself will not be removed by this request. + * </p> + * <p> + * Note that the number of values in a property (e.g., {@link Property#size()}, {@link Property#isEmpty()}, + * {@link Property#isSingle()}, and {@link Property#isMultiple()}) has no influence on whether the property should be removed. It + * is possible for a property to have no values. + * </p> + */ +public class UpdateValuesRequest extends ChangeRequest { + + private static final long serialVersionUID = 1L; + + private final String workspaceName; + private final Location on; + private final Name propertyName; + private final List<Object> addedValues; + private final List<Object> removedValues; + + private Location actualLocation; + private List<Object> actualAddedValues; + private List<Object> actualRemovedValues; + + + public UpdateValuesRequest( String workspaceName, + Location on, + Name propertyName, + List<Object> addedValues, + List<Object> removedValues ) { + super(); + + assert workspaceName != null; + assert on != null; + assert propertyName != null; + + this.workspaceName = workspaceName; + this.on = on; + this.propertyName = propertyName; + this.addedValues = addedValues == null ? Collections.emptyList() : addedValues; + this.removedValues = removedValues == null ? Collections.emptyList() : removedValues; + } + + /** + * Get the location defining the node that is to be updated. + * + * @return the location of the node; never null + */ + public Location on() { + return on; + } + + /** + * Get the name of the property that is to be updated. + * + * @return the name of the property; never null + */ + public Name property() { + return propertyName; + } + + /** + * Get the name of the workspace in which the node exists. + * + * @return the name of the workspace; never null + */ + public String inWorkspace() { + return workspaceName; + } + + /** + * Get the list of values to be added. + * + * @return the values (if any) to be added; never null + */ + public List<Object> addedValues() { + return addedValues; + } + + /** + * Get the list of values to be removed. + * + * @return the values (if any) to be removed; never null + */ + public List<Object> removedValues() { + return removedValues; + } + + @Override + public Location changedLocation() { + return on; + } + + @Override + public String changedWorkspace() { + return workspaceName; + } + + @Override + public boolean changes( String workspace, + Path path ) { + return workspaceName.equals(workspace) && on.hasPath() && on.getPath().equals(path); + } + + @Override + public boolean isReadOnly() { + return addedValues.isEmpty() && removedValues.isEmpty(); + } + + public void setActualLocation(Location actual, List<Object> actualAddedValues, List<Object> actualRemovedValues) { + checkNotFrozen(); + if (!on.isSame(actual)) { // not same if actual is null + throw new IllegalArgumentException(GraphI18n.actualLocationIsNotSameAsInputLocation.text(actual, on)); + } + assert actual != null; + if (!actual.hasPath()) { + throw new IllegalArgumentException(GraphI18n.actualLocationMustHavePath.text(actual)); + } + this.actualLocation = actual; + assert actualLocation != null; + + assert actualAddedValues != null; + assert actualAddedValues.size() <= addedValues.size(); + assert actualRemovedValues != null; + assert actualRemovedValues.size() <= actualRemovedValues.size(); + + this.actualAddedValues = actualAddedValues; + this.actualRemovedValues = actualRemovedValues; + } + + /** + * Get the actual location of the node that was updated. + * + * @return the actual location, or null if the actual location was not set + */ + public Location getActualLocationOfNode() { + return actualLocation; + } + + /** + * Get the actual added values. This should always be identical to the list of values that were requested to be added. + * + * @return the values that were added to the node when this request was processed; never null + */ + public List<Object> getActualAddedValues() { + return actualAddedValues; + } + + /** + * Get the actual removed values. This will differ from the values that were requested to be removed if some of the values + * that were requested to be removed were not already values for the property. + * + * @return the values that were removed from the node when this request was processed; never null + */ + public List<Object> getActualRemovedValues() { + return actualRemovedValues; + } +} Property changes on: trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/UpdateValuesRequest.java ___________________________________________________________________ Name: svn:keywords + Id Revision Name: svn:eol-style + LF Modified: trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/processor/RequestProcessor.java =================================================================== --- trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/processor/RequestProcessor.java 2009-07-26 04:10:00 UTC (rev 1135) +++ trunk/dna-graph/src/main/java/org/jboss/dna/graph/request/processor/RequestProcessor.java 2009-07-26 16:33:57 UTC (rev 1136) @@ -23,7 +23,10 @@ */ package org.jboss.dna.graph.request.processor; +import java.util.ArrayList; +import java.util.Arrays; import java.util.Collections; +import java.util.Iterator; import java.util.LinkedList; import java.util.List; import java.util.Map; @@ -68,6 +71,7 @@ import org.jboss.dna.graph.request.SetPropertyRequest; import org.jboss.dna.graph.request.UnsupportedRequestException; import org.jboss.dna.graph.request.UpdatePropertiesRequest; +import org.jboss.dna.graph.request.UpdateValuesRequest; import org.jboss.dna.graph.request.VerifyNodeExistsRequest; import org.jboss.dna.graph.request.VerifyWorkspaceRequest; @@ -259,6 +263,8 @@ process((CloneWorkspaceRequest)request); } else if (request instanceof DestroyWorkspaceRequest) { process((DestroyWorkspaceRequest)request); + } else if (request instanceof UpdateValuesRequest) { + process((UpdateValuesRequest)request); } else { processUnknownRequest(request); } @@ -749,6 +755,59 @@ public abstract void process( UpdatePropertiesRequest request ); /** + * Process a request to add and/or remove the specified values from a property on the given node. + * <p> + * This method does nothing if the request is null. + * </p> + * + * @param request the remove request + */ + public void process( UpdateValuesRequest request ) { + String workspaceName = request.inWorkspace(); + Location on = request.on(); + Name propertyName = request.property(); + + // Read in the current values + ReadPropertyRequest readProperty = new ReadPropertyRequest(on, workspaceName, propertyName); + process(readProperty); + + if (readProperty.hasError()) { + request.setError(readProperty.getError()); + return; + } + + Property property = readProperty.getProperty(); + List<Object> actualRemovedValues = new ArrayList<Object>(request.removedValues().size()); + List<Object> newValues = property == null ? new LinkedList<Object>() : new LinkedList<Object>( + Arrays.asList(property.getValuesAsArray())); + // Calculate what the new values should be + for (Object removedValue : request.removedValues()) { + for (Iterator<Object> iter = newValues.iterator(); iter.hasNext();) { + if (iter.next().equals(removedValue)) { + iter.remove(); + actualRemovedValues.add(removedValue); + break; + } + } + } + + newValues.addAll(request.addedValues()); + Property newProperty = getExecutionContext().getPropertyFactory().create(propertyName, newValues); + + // Update the current values + SetPropertyRequest setProperty = new SetPropertyRequest(on, workspaceName, newProperty); + process(setProperty); + + if (setProperty.hasError()) { + request.setError(setProperty.getError()); + } else { + // Set the actual location ... + request.setActualLocation(setProperty.getActualLocationOfNode(), request.addedValues(), actualRemovedValues); + } + + } + + /** * Process a request to rename a node specified location into a different location. * <p> * This method does nothing if the request is null. Unless overridden, this method converts the rename into a 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-07-26 04:10:00 UTC (rev 1135) +++ trunk/dna-graph/src/test/java/org/jboss/dna/graph/connector/test/WritableConnectorTest.java 2009-07-26 16:33:57 UTC (rev 1136) @@ -1849,4 +1849,101 @@ "The quick brown fox jumped over the moon. What? ")); } + @Test + public void shouldAddValuesToExistingProperty() { + + String initialPath = ""; + int depth = 1; + int numChildrenPerNode = 1; + int numPropertiesPerNode = 1; + 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())); + + assertThat(subgraph.getNode("node1"), hasProperty("property1", "The quick brown fox jumped over the moon. What? ")); + + graph.addValue("foo").andValue("bar").to("property1").on("node1"); + + assertThat(graph.getChildren().of("/"), hasChildren(segment("node1"))); + subgraph = graph.getSubgraphOfDepth(2).at("/"); + assertThat(subgraph, is(notNullValue())); + + assertThat(subgraph.getNode("node1"), hasProperty("property1", + "The quick brown fox jumped over the moon. What? ", + "foo", + "bar")); + } + + @Test + public void shouldAddValuesToNonExistantProperty() { + + String initialPath = ""; + int depth = 1; + int numChildrenPerNode = 1; + int numPropertiesPerNode = 1; + 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"); + + assertThat(graph.getChildren().of("/"), hasChildren(segment("node1"))); + Subgraph subgraph = graph.getSubgraphOfDepth(2).at("/"); + assertThat(subgraph, is(notNullValue())); + + assertThat(subgraph.getNode("node1"), hasProperty("property1", "The quick brown fox jumped over the moon. What? ")); + assertThat(subgraph.getNode("node1"), hasProperty("newProperty", "foo", "bar")); + + } + + @Test + public void shouldRemoveValuesFromExistingProperty() { + + String initialPath = ""; + int depth = 1; + int numChildrenPerNode = 1; + int numPropertiesPerNode = 1; + 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"); + + assertThat(graph.getChildren().of("/"), hasChildren(segment("node1"))); + Subgraph subgraph = graph.getSubgraphOfDepth(2).at("/"); + assertThat(subgraph, is(notNullValue())); + + assertThat(subgraph.getNode("node1"), hasProperty("property1")); + } + + @Test + public void shouldNotRemoveValuesFromNonExistantProperty() { + + String initialPath = ""; + int depth = 1; + int numChildrenPerNode = 1; + int numPropertiesPerNode = 1; + 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"); + + assertThat(graph.getChildren().of("/"), hasChildren(segment("node1"))); + Subgraph subgraph = graph.getSubgraphOfDepth(2).at("/"); + assertThat(subgraph, is(notNullValue())); + + assertThat(subgraph.getNode("node1"), hasProperty("property1", "The quick brown fox jumped over the moon. What? ")); + } + }