TINKERPOP-1642 Made Mutating steps implement Scoping. This simplified PathUtil.getReferencedLabels() and reduced iterations over Parameter traversals.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/1f99a510 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/1f99a510 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/1f99a510 Branch: refs/heads/master Commit: 1f99a5103bfaec758d657078362ac666fe47e8d2 Parents: 88a3f60 Author: Stephen Mallette <sp...@genoprime.com> Authored: Fri Mar 10 15:11:52 2017 -0500 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Wed Mar 29 11:21:06 2017 -0400 ---------------------------------------------------------------------- CHANGELOG.asciidoc | 1 + .../process/traversal/Parameterizing.java | 8 +++++ .../gremlin/process/traversal/step/Scoping.java | 14 +++++--- .../process/traversal/step/map/AddEdgeStep.java | 11 +++++- .../traversal/step/map/AddVertexStartStep.java | 9 ++++- .../traversal/step/map/AddVertexStep.java | 9 ++++- .../step/sideEffect/AddPropertyStep.java | 9 ++++- .../process/traversal/step/util/Parameters.java | 37 +++++++++++++++++--- .../process/traversal/util/PathUtil.java | 17 +-------- 9 files changed, 85 insertions(+), 30 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/1f99a510/CHANGELOG.asciidoc ---------------------------------------------------------------------- diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 6e88565..8dd8d9a 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima TinkerPop 3.2.5 (Release Date: NOT OFFICIALLY RELEASED YET) ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ +* `Mutating` steps now implement `Scoping` interface. * Fixed a step id compilation bug in `AddVertexStartStep`, `AddVertexStep`, `AddEdgeStep`, and `AddPropertyStep`. * De-registered metrics on Gremlin Server shutdown. * Added "help" command option on `:remote config` for plugins that support that feature in the Gremlin Console. http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/1f99a510/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Parameterizing.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Parameterizing.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Parameterizing.java index 8af80b3..9b7e088 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Parameterizing.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Parameterizing.java @@ -21,9 +21,17 @@ package org.apache.tinkerpop.gremlin.process.traversal; import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters; /** + * An interface for {@link Step} implementations that hold a {@link Parameters} object, which fold in arguments from + * other steps. It is typically used on mutating steps, such as {@code addV()}, where calls to {@code property(k,v)} + * are folded in as parameters to that add vertex step. + * * @author Marko A. Rodriguez (http://markorodriguez.com) + * @author Stephen Mallette (http://stephen.genoprime.com) */ public interface Parameterizing { + /** + * Gets the parameters on the step. + */ public Parameters getParameters(); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/1f99a510/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java index 22109bf..683e661 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/Scoping.java @@ -20,23 +20,27 @@ package org.apache.tinkerpop.gremlin.process.traversal.step; import org.apache.tinkerpop.gremlin.process.traversal.Path; import org.apache.tinkerpop.gremlin.process.traversal.Pop; +import org.apache.tinkerpop.gremlin.process.traversal.Step; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; -import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement; -import java.util.EnumSet; import java.util.Map; import java.util.Set; /** + * This interface is implemented by {@link Step} implementations that access labeled path steps, side-effects or + * {@code Map} values by key, such as {@code select('a')} step. Note that a step like {@code project()} is non-scoping + * because while it creates a {@code Map} it does not introspect them. + * * @author Marko A. Rodriguez (http://markorodriguez.com) + * @author Stephen Mallette (http://stephen.genoprime.com) */ public interface Scoping { - public static enum Variable {START, END} + public enum Variable {START, END} public default <S> S getScopeValue(final Pop pop, final String key, final Traverser.Admin<?> traverser) throws IllegalArgumentException { if (traverser.getSideEffects().exists(key)) - return traverser.getSideEffects().<S>get(key); + return traverser.getSideEffects().get(key); /// final Object object = traverser.get(); if (object instanceof Map && ((Map<String, S>) object).containsKey(key)) @@ -51,7 +55,7 @@ public interface Scoping { public default <S> S getNullableScopeValue(final Pop pop, final String key, final Traverser.Admin<?> traverser) { if (traverser.getSideEffects().exists(key)) - return traverser.getSideEffects().<S>get(key); + return traverser.getSideEffects().get(key); /// final Object object = traverser.get(); if (object instanceof Map && ((Map<String, S>) object).containsKey(key)) http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/1f99a510/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java index 13efc8e..3a46c0d 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddEdgeStep.java @@ -23,6 +23,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.gremlin.process.traversal.step.FromToModulating; import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating; +import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping; import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters; import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.CallbackRegistry; @@ -36,6 +37,8 @@ import org.apache.tinkerpop.gremlin.structure.Vertex; import org.apache.tinkerpop.gremlin.structure.util.StringFactory; import org.apache.tinkerpop.gremlin.structure.util.detached.DetachedFactory; +import java.util.HashSet; +import java.util.Iterator; import java.util.List; import java.util.Set; @@ -43,7 +46,8 @@ import java.util.Set; * @author Marko A. Rodriguez (http://markorodriguez.com) * @author Stephen Mallette (http://stephen.genoprime.com) */ -public final class AddEdgeStep<S> extends MapStep<S, Edge> implements Mutating<Event.EdgeAddedEvent>, TraversalParent, Parameterizing, FromToModulating { +public final class AddEdgeStep<S> extends MapStep<S, Edge> + implements Mutating<Event.EdgeAddedEvent>, TraversalParent, Parameterizing, Scoping, FromToModulating { private static final String FROM = Graph.Hidden.hide("from"); private static final String TO = Graph.Hidden.hide("to"); @@ -67,6 +71,11 @@ public final class AddEdgeStep<S> extends MapStep<S, Edge> implements Mutating<E } @Override + public Set<String> getScopeKeys() { + return this.parameters.getReferencedLabels(); + } + + @Override public void addPropertyMutations(final Object... keyValues) { this.parameters.set(keyValues); this.parameters.integrateTraversals(this); http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/1f99a510/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java index e7939dc..064fc79 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStartStep.java @@ -24,6 +24,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.gremlin.process.traversal.TraverserGenerator; import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating; +import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping; import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; import org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep; import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters; @@ -44,7 +45,8 @@ import java.util.Set; * @author Marko A. Rodriguez (http://markorodriguez.com) * @author Stephen Mallette (http://stephen.genoprime.com) */ -public final class AddVertexStartStep extends AbstractStep<Vertex, Vertex> implements Mutating<Event.VertexAddedEvent>, TraversalParent, Parameterizing { +public final class AddVertexStartStep extends AbstractStep<Vertex, Vertex> + implements Mutating<Event.VertexAddedEvent>, TraversalParent, Parameterizing, Scoping { private Parameters parameters = new Parameters(); private boolean first = true; @@ -62,6 +64,11 @@ public final class AddVertexStartStep extends AbstractStep<Vertex, Vertex> imple } @Override + public Set<String> getScopeKeys() { + return this.parameters.getReferencedLabels(); + } + + @Override public <S, E> List<Traversal.Admin<S, E>> getLocalChildren() { return this.parameters.getTraversals(); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/1f99a510/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java index 57cd616..69ee6e5 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AddVertexStep.java @@ -22,6 +22,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Parameterizing; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating; +import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping; import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters; import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.CallbackRegistry; @@ -40,7 +41,8 @@ import java.util.Set; * @author Marko A. Rodriguez (http://markorodriguez.com) * @author Stephen Mallette (http://stephen.genoprime.com) */ -public final class AddVertexStep<S> extends MapStep<S, Vertex> implements Mutating<Event.VertexAddedEvent>, TraversalParent, Parameterizing { +public final class AddVertexStep<S> extends MapStep<S, Vertex> + implements Mutating<Event.VertexAddedEvent>, TraversalParent, Parameterizing, Scoping { private Parameters parameters = new Parameters(); private CallbackRegistry<Event.VertexAddedEvent> callbackRegistry; @@ -57,6 +59,11 @@ public final class AddVertexStep<S> extends MapStep<S, Vertex> implements Mutati } @Override + public Set<String> getScopeKeys() { + return this.parameters.getReferencedLabels(); + } + + @Override public <S, E> List<Traversal.Admin<S, E>> getLocalChildren() { return this.parameters.getTraversals(); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/1f99a510/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java index 44232f0..85292b7 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStep.java @@ -22,6 +22,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.Parameterizing; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating; +import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping; import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters; import org.apache.tinkerpop.gremlin.process.traversal.step.util.event.CallbackRegistry; @@ -45,7 +46,8 @@ import java.util.Set; /** * @author Marko A. Rodriguez (http://markorodriguez.com) */ -public final class AddPropertyStep<S extends Element> extends SideEffectStep<S> implements Mutating<Event.ElementPropertyChangedEvent>, TraversalParent, Parameterizing { +public final class AddPropertyStep<S extends Element> extends SideEffectStep<S> + implements Mutating<Event.ElementPropertyChangedEvent>, TraversalParent, Parameterizing, Scoping { private Parameters parameters = new Parameters(); private final VertexProperty.Cardinality cardinality; @@ -65,6 +67,11 @@ public final class AddPropertyStep<S extends Element> extends SideEffectStep<S> } @Override + public Set<String> getScopeKeys() { + return this.parameters.getReferencedLabels(); + } + + @Override public <S, E> List<Traversal.Admin<S, E>> getLocalChildren() { return this.parameters.getTraversals(); } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/1f99a510/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java index 93cf1f8..4b38d2e 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/Parameters.java @@ -22,6 +22,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.step.util; import org.apache.commons.lang.ArrayUtils; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; +import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping; import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalUtil; import org.apache.tinkerpop.gremlin.structure.Element; @@ -33,8 +34,10 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Map; +import java.util.Set; import java.util.function.Supplier; /** @@ -48,6 +51,7 @@ public final class Parameters implements Cloneable, Serializable { private static final Object[] EMPTY_ARRAY = new Object[0]; private Map<Object, List<Object>> parameters = new HashMap<>(); + private Set<String> referencedLabels = new HashSet<>(); /** * A cached list of traversals that serve as parameter values. The list is cached on calls to @@ -111,14 +115,15 @@ public final class Parameters implements Cloneable, Serializable { public Object remove(final Object key) { final List<Object> o = parameters.remove(key); - // once a key is removed, it's possible that the traversal cache will need to be regenerated + // once a key is removed, it's possible that the traversal/label cache will need to be regenerated if (IteratorUtils.anyMatch(o.iterator(), p -> p instanceof Traversal.Admin)) { traversals.clear(); traversals = new ArrayList<>(); for (final List<Object> list : this.parameters.values()) { for (final Object object : list) { if (object instanceof Traversal.Admin) { - traversals.add((Traversal.Admin) object); + final Traversal.Admin t = (Traversal.Admin) object; + addTraversal(t); } } } @@ -170,9 +175,11 @@ public final class Parameters implements Cloneable, Serializable { for (int i = 0; i < keyValues.length; i = i + 2) { if (keyValues[i + 1] != null) { // track the list of traversals that are present so that elsewhere in Parameters there is no need - // to iterate all values to not find any. - if (keyValues[i + 1] instanceof Traversal.Admin) - traversals.add((Traversal.Admin) keyValues[i + 1]); + // to iterate all values to not find any. also grab available labels in traversal values + if (keyValues[i + 1] instanceof Traversal.Admin) { + final Traversal.Admin t = (Traversal.Admin) keyValues[i + 1]; + addTraversal(t); + } List<Object> values = this.parameters.get(keyValues[i]); if (null == values) { @@ -206,6 +213,13 @@ public final class Parameters implements Cloneable, Serializable { return (List<Traversal.Admin<S, E>>) (Object) traversals; } + /** + * Gets a list of all labels held in parameters that have a traversal as a value. + */ + public Set<String> getReferencedLabels() { + return referencedLabels; + } + public Parameters clone() { try { final Parameters clone = (Parameters) super.clone(); @@ -217,6 +231,8 @@ public final class Parameters implements Cloneable, Serializable { } clone.parameters.put(entry.getKey() instanceof Traversal.Admin ? ((Traversal.Admin) entry.getKey()).clone() : entry.getKey(), values); } + + clone.referencedLabels = new HashSet<>(this.referencedLabels); return clone; } catch (final CloneNotSupportedException e) { throw new IllegalStateException(e.getMessage(), e); @@ -246,4 +262,15 @@ public final class Parameters implements Cloneable, Serializable { throw new IllegalArgumentException("The provided key/value array must have a String, T, or Traversal on even array indices"); } } + + private void addTraversal(final Traversal.Admin t) { + traversals.add(t); + for (final Object ss : t.getSteps()) { + if (ss instanceof Scoping) { + for (String label : ((Scoping) ss).getScopeKeys()) { + referencedLabels.add(label); + } + } + } + } } http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/1f99a510/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/PathUtil.java ---------------------------------------------------------------------- diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/PathUtil.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/PathUtil.java index ab97836..cefc62a 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/PathUtil.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/PathUtil.java @@ -18,19 +18,17 @@ */ package org.apache.tinkerpop.gremlin.process.traversal.util; -import org.apache.tinkerpop.gremlin.process.traversal.Parameterizing; import org.apache.tinkerpop.gremlin.process.traversal.Step; -import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping; import org.apache.tinkerpop.gremlin.process.traversal.step.map.MatchStep; import org.apache.tinkerpop.gremlin.process.traversal.step.util.EmptyStep; -import org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters; import java.util.HashSet; import java.util.Set; /** * @author Ted Wilmes (http://twilmes.org) + * @author Stephen Mallette (http://stephen.genoprime.com) */ public class PathUtil { @@ -50,19 +48,6 @@ public class PathUtil { public static Set<String> getReferencedLabels(final Step step) { final Set<String> referencedLabels = new HashSet<>(); - if (step instanceof Parameterizing) { // TODO: we should really make the mutation steps Scoping :| - final Parameters parameters = ((Parameterizing) step).getParameters(); - for (final Traversal.Admin trav : parameters.getTraversals()) { - for (final Object ss : trav.getSteps()) { - if (ss instanceof Scoping) { - for (String label : ((Scoping) ss).getScopeKeys()) { - referencedLabels.add(label); - } - } - } - } - } - if (step instanceof Scoping) { final Set<String> labels = new HashSet<>(((Scoping) step).getScopeKeys()); if (step instanceof MatchStep) {