This is an automated email from the ASF dual-hosted git repository. colegreer pushed a commit to branch traversal-parents in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 9b292c63de59dfa52f886aabc8e8e9d03b1efca4 Author: Cole-Greer <[email protected]> AuthorDate: Fri Sep 12 13:37:58 2025 -0700 Fix TraversalParent implementations for Placeholder steps The TraversalParent methods were not consistently implemented in the new placeholder steps. This commit adds proper implementations in all TraversalParent placeholder steps, and introduces TraversalParentTest. TraversalParentTest takes traversals with TraversalParents and verifies that getGlobalChildren() and getLocalChildren() are producing the expected children, and that those children have correctly set their parents. These checks are done before and after strategy application. This is primarily intended for placeholder steps to ensure that TraversalParent remains correctly implemented in both placeholder and concrete steps, however it is also useful to verify against interference from strategies. The new tests identified an existing deficiency DateDiffStep which was also addressed --- .../traversal/lambda/GValueConstantTraversal.java | 11 + .../process/traversal/step/TraversalParent.java | 18 + .../step/map/AbstractAddEdgeStepPlaceholder.java | 26 +- .../map/AbstractAddElementStepPlaceholder.java | 26 +- .../map/AbstractMergeElementStepPlaceholder.java | 50 +- .../process/traversal/step/map/DateDiffStep.java | 9 + .../traversal/step/map/TraversalSelectStep.java | 2 +- .../traversal/step/sideEffect/AddPropertyStep.java | 6 +- .../step/sideEffect/AddPropertyStepContract.java | 12 +- .../sideEffect/AddPropertyStepPlaceholder.java | 85 +- .../traversal/step/TraversalParentTest.java | 1014 ++++++++++++++++++++ .../step/sideEffect/AddPropertyStepTest.java | 2 +- 12 files changed, 1205 insertions(+), 56 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/GValueConstantTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/GValueConstantTraversal.java index a82d095fe7..a0fbc9f636 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/GValueConstantTraversal.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/lambda/GValueConstantTraversal.java @@ -57,6 +57,17 @@ public final class GValueConstantTraversal<S, E> extends AbstractLambdaTraversal return constantTraversal.equals(other); } + @Override + public GValueConstantTraversal<S, E> clone() { + GValueConstantTraversal<S, E> clone = new GValueConstantTraversal<>(GValue.of(this.end.getName(), this.end.get())); + try { + clone.setGValueManager(this.getGValueManager().clone()); + } catch (CloneNotSupportedException e) { //TODO:: handle properly + throw new RuntimeException(e); + } + return clone; + } + public ConstantTraversal<S, E> getConstantTraversal() { return constantTraversal; } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParent.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParent.java index 0e064fd40f..275c4f718c 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParent.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParent.java @@ -21,6 +21,8 @@ package org.apache.tinkerpop.gremlin.process.traversal.step; import org.apache.tinkerpop.gremlin.process.traversal.GValueManager; import org.apache.tinkerpop.gremlin.process.traversal.Step; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.step.branch.BranchStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.branch.LocalStep; import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement; import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; @@ -31,14 +33,30 @@ import java.util.List; import java.util.Set; /** + * Indicates that a step can contain child Traversals. Any step which implements this interface should override at least + * one of getGlobalChildren() or getLocalChildren(). All TraversalParents should call integrateChild() for all child + * traversals as they are added to the step, and TraversalParents should override {@link Step::setTraversal()}, and + * call integrateChild() again for all child traversals. + * * @author Marko A. Rodriguez (http://markorodriguez.com) */ public interface TraversalParent extends PopContaining, AutoCloseable { + /** + * Gets a list of all "global" child traversals for this step. A "global" traversal is one which evaluates across + * all of its parents incoming traversers at once. This is typically used in cases where the child traversal + * represents a branch of traversal flow. See {@link BranchStep} as an example. + */ public default <S, E> List<Traversal.Admin<S, E>> getGlobalChildren() { return Collections.emptyList(); } + /** + * Gets a list of all "local" child traversals for this step. A "local" traversal is one which is evaluated + * independently for each incoming traverser to the parent step. This is typically used in cases where the child + * is used to process or augment each traverser individually. See {@link LocalStep} or {@link ByModulating} as + * examples. + */ public default <S, E> List<Traversal.Admin<S, E>> getLocalChildren() { return Collections.emptyList(); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractAddEdgeStepPlaceholder.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractAddEdgeStepPlaceholder.java index 80c5c766c4..a5d9c46600 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractAddEdgeStepPlaceholder.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractAddEdgeStepPlaceholder.java @@ -65,20 +65,6 @@ public abstract class AbstractAddEdgeStepPlaceholder<S> extends AbstractAddEleme this.integrateChild(this.from); } - @Override - public List<Traversal.Admin<S, Edge>> getLocalChildren() { - final List<Traversal.Admin<S, Edge>> childTraversals = super.getLocalChildren(); - if (from != null) childTraversals.add((Traversal.Admin) from); - if (to != null) childTraversals.add((Traversal.Admin) to); - return childTraversals; - } - - @Override - public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { - super.setTraversal(parentTraversal); - this.getLocalChildren().forEach(this::integrateChild); - } - @Override public int hashCode() { int hash = super.hashCode(); @@ -101,6 +87,18 @@ public abstract class AbstractAddEdgeStepPlaceholder<S> extends AbstractAddEleme return false; } + @Override + public List<Traversal.Admin<?, ?>> getLocalChildren() { + List<Traversal.Admin<?, ?>> childTraversals = super.getLocalChildren(); + if (from != null) { + childTraversals.add(from instanceof GValueConstantTraversal ? ((GValueConstantTraversal<?, ?>) from).getConstantTraversal() : from); + } + if (to != null) { + childTraversals.add(to instanceof GValueConstantTraversal ? ((GValueConstantTraversal<?, ?>) to).getConstantTraversal() : to); + } + return childTraversals; + } + @Override protected boolean supportsMultiProperties() { return false; diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractAddElementStepPlaceholder.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractAddElementStepPlaceholder.java index 401ea0ece9..c166c14eaf 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractAddElementStepPlaceholder.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractAddElementStepPlaceholder.java @@ -81,22 +81,35 @@ public abstract class AbstractAddElementStepPlaceholder<S, E extends Element, X } protected void addTraversal(final Traversal.Admin<?, ?> traversal) { + integrateChild(traversal); TraversalHelper.getStepsOfAssignableClassRecursively(Scoping.class, traversal).forEach(s -> scopeKeys.addAll(s.getScopeKeys())); } @Override - public List<Traversal.Admin<S, E>> getLocalChildren() { - List<Traversal.Admin<S, E>> childTraversals = new ArrayList<>(); - for (List<Object> values : properties.values()) { - for (Object value : values) { + public List<Traversal.Admin<?, ?>> getLocalChildren() { + List<Traversal.Admin<?, ?>> childTraversals = new ArrayList<>(); + for (Map.Entry<Object, List<Object>> entry : properties.entrySet()) { + if (entry.getKey() instanceof Traversal) { + childTraversals.add((Traversal.Admin<?, ?>) ((Traversal) entry.getKey()).asAdmin()); + } + for (Object value : entry.getValue()) { if (value instanceof Traversal) { - childTraversals.add((Traversal.Admin<S, E>) ((Traversal) value).asAdmin()); + childTraversals.add((Traversal.Admin<?, ?>) ((Traversal) value).asAdmin()); } } } + if (label != null) { + childTraversals.add(label); + } return childTraversals; } + @Override + public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { + super.setTraversal(parentTraversal); + this.getLocalChildren().forEach(this::integrateChild); + } + @Override public Set<TraverserRequirement> getRequirements() { return this.getSelfAndChildRequirements(TraverserRequirement.OBJECT); @@ -175,6 +188,9 @@ public abstract class AbstractAddElementStepPlaceholder<S, E extends Element, X if (key instanceof GValue) { throw new IllegalArgumentException("GValue cannot be used as a property key"); } + if (key instanceof Traversal) { + this.integrateChild(((Traversal<?, ?>) key).asAdmin()); + } if (value instanceof GValue) { traversal.getGValueManager().register((GValue<?>) value); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractMergeElementStepPlaceholder.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractMergeElementStepPlaceholder.java index 0dba2bbeb8..45b475b13b 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractMergeElementStepPlaceholder.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/AbstractMergeElementStepPlaceholder.java @@ -33,6 +33,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.Partit import org.apache.tinkerpop.gremlin.process.traversal.traverser.TraverserRequirement; import org.apache.tinkerpop.gremlin.structure.T; +import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; import java.util.Collections; @@ -55,6 +56,7 @@ public abstract class AbstractMergeElementStepPlaceholder<S, E> extends Abstract public AbstractMergeElementStepPlaceholder(Traversal.Admin traversal, final Traversal.Admin<?, Map<Object, Object>> mergeTraversal, final boolean isStart) { super(traversal); this.mergeTraversal = mergeTraversal; + this.integrateChild(mergeTraversal); this.isStart = isStart; } @@ -73,6 +75,27 @@ public abstract class AbstractMergeElementStepPlaceholder<S, E> extends Abstract return super.getRequirements(); } + @Override + public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { + super.setTraversal(parentTraversal); + this.getLocalChildren().forEach(this::integrateChild); + } + + @Override + public List<Traversal.Admin<?, ?>> getLocalChildren() { + List<Traversal.Admin<?, ?>> localChildren = new ArrayList<>(); + if (mergeTraversal != null) { + localChildren.add(mergeTraversal); + } + if (onCreateTraversal != null) { + localChildren.add(onCreateTraversal); + } + if (onMatchTraversal != null) { + localChildren.add(onMatchTraversal); + } + return localChildren; + } + @Override public Traversal.Admin getMergeTraversal() { if (mergeTraversal != null && mergeTraversal instanceof GValueConstantTraversal) { @@ -158,27 +181,30 @@ public abstract class AbstractMergeElementStepPlaceholder<S, E> extends Abstract } @Override - public void setOnMatch(final Traversal.Admin<?, Map<Object, Object>> onMatchMap) { - this.onMatchTraversal = onMatchMap; - if (onMatchMap instanceof GValueConstantTraversal && ((GValueConstantTraversal<?, Map<Object, Object>>) onMatchMap).isParameterized()) { - traversal.getGValueManager().register(((GValueConstantTraversal<?, Map<Object, Object>>) onMatchMap).getGValue()); + public void setOnMatch(final Traversal.Admin<?, Map<Object, Object>> onMatchTraversal) { + this.onMatchTraversal = onMatchTraversal; + if (onMatchTraversal instanceof GValueConstantTraversal && ((GValueConstantTraversal<?, Map<Object, Object>>) onMatchTraversal).isParameterized()) { + traversal.getGValueManager().register(((GValueConstantTraversal<?, Map<Object, Object>>) onMatchTraversal).getGValue()); } + this.integrateChild(onMatchTraversal); } @Override - public void setOnCreate(final Traversal.Admin<?, Map<Object, Object>> onCreateMap) { - this.onCreateTraversal = onCreateMap; - if (onCreateMap instanceof GValueConstantTraversal && ((GValueConstantTraversal<?, Map<Object, Object>>) onCreateMap).isParameterized()) { - traversal.getGValueManager().register(((GValueConstantTraversal<?, Map<Object, Object>>) onCreateMap).getGValue()); + public void setOnCreate(final Traversal.Admin<?, Map<Object, Object>> onCreateTraversal) { + this.onCreateTraversal = onCreateTraversal; + if (onCreateTraversal instanceof GValueConstantTraversal && ((GValueConstantTraversal<?, Map<Object, Object>>) onCreateTraversal).isParameterized()) { + traversal.getGValueManager().register(((GValueConstantTraversal<?, Map<Object, Object>>) onCreateTraversal).getGValue()); } + this.integrateChild(onCreateTraversal); } @Override - public void setMerge(Traversal.Admin<?, Map<Object, Object>> mergeMap) { - this.mergeTraversal = mergeMap; - if (mergeMap instanceof GValueConstantTraversal && ((GValueConstantTraversal<?, Map<Object, Object>>) mergeMap).isParameterized()) { - traversal.getGValueManager().register(((GValueConstantTraversal<?, Map<Object, Object>>) mergeMap).getGValue()); + public void setMerge(Traversal.Admin<?, Map<Object, Object>> mergeTraversal) { + this.mergeTraversal = mergeTraversal; + if (mergeTraversal instanceof GValueConstantTraversal && ((GValueConstantTraversal<?, Map<Object, Object>>) mergeTraversal).isParameterized()) { + traversal.getGValueManager().register(((GValueConstantTraversal<?, Map<Object, Object>>) mergeTraversal).getGValue()); } + this.integrateChild(mergeTraversal); } @Override diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/DateDiffStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/DateDiffStep.java index 9f1f37fdbc..08b1789760 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/DateDiffStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/DateDiffStep.java @@ -30,6 +30,7 @@ import java.time.OffsetDateTime; import java.time.ZoneOffset; import java.util.Collections; import java.util.Date; +import java.util.List; import java.util.Set; /** @@ -102,9 +103,17 @@ public final class DateDiffStep<S> extends ScalarMapStep<S, Long> implements Tra return Collections.singleton(TraverserRequirement.OBJECT); } + @Override + public List<Traversal.Admin<?, ?>> getLocalChildren() { + return dateTraversal == null ? Collections.emptyList() : List.of(dateTraversal); + } + @Override public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { super.setTraversal(parentTraversal); + if (dateTraversal != null) { + integrateChild(dateTraversal); + } } @Override diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TraversalSelectStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TraversalSelectStep.java index 0eea87f36c..46ab0a3a79 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TraversalSelectStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/TraversalSelectStep.java @@ -114,7 +114,7 @@ public final class TraversalSelectStep<S, E> extends MapStep<S, E> implements Tr @Override public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { super.setTraversal(parentTraversal); - this.integrateChild(this.selectTraversal); + this.getLocalChildren().forEach(this::integrateChild); } @Override 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 feb02b98d8..be73f3581a 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 @@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.ConstantTraversal; import org.apache.tinkerpop.gremlin.process.traversal.step.Configuring; import org.apache.tinkerpop.gremlin.process.traversal.step.Deleting; import org.apache.tinkerpop.gremlin.process.traversal.step.Writing; @@ -209,7 +210,10 @@ public class AddPropertyStep<S extends Element> extends SideEffectStep<S> @Override public Object getValue() { List<Object> values = parameters.get(T.value, null); - return values.isEmpty() ? null : values.get(0); + if (values.isEmpty()) { + return null; + } + return values.get(0) instanceof ConstantTraversal ? ((ConstantTraversal<?, ?>) values.get(0)).next() : values.get(0); } @Override diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepContract.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepContract.java index f7f0691a92..a246e2aec4 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepContract.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepContract.java @@ -19,6 +19,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect; import org.apache.tinkerpop.gremlin.process.traversal.Step; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.ConstantTraversal; import org.apache.tinkerpop.gremlin.process.traversal.step.Deleting; import org.apache.tinkerpop.gremlin.process.traversal.step.GValue; import org.apache.tinkerpop.gremlin.process.traversal.step.Scoping; @@ -47,14 +48,17 @@ public interface AddPropertyStepContract<S> extends Step<S, S>, TraversalParent, Object getKey(); /** - * Get the property value + * Gets the property value. If the value was originally passed as a {@link GValue<?>}, {@link ConstantTraversal <?>}, + * or a literal value, then the literal value is returned. Otherwise, the MergeMap is returned in Traversal form. */ Object getValue(); /** - * Get the value as a GValue, without pinning the variable + * Gets the property value. If the value was originally passed as a {@link GValue<?>}, that is returned + * directly. If it was originally passed as a {@link ConstantTraversal <?>}, or a literal value, then the literal + * value is returned. Otherwise, the MergeMap is returned in Traversal form. */ - default GValue<?> getValueAsGValue() { - return GValue.of(getValue()); + default Object getValueWithGValue() { + return getValue(); } } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepPlaceholder.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepPlaceholder.java index af3cb92846..1a0889c246 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepPlaceholder.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepPlaceholder.java @@ -20,6 +20,8 @@ package org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect; import org.apache.tinkerpop.gremlin.process.traversal.Traversal; import org.apache.tinkerpop.gremlin.process.traversal.Traverser; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.ConstantTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.GValueConstantTraversal; import org.apache.tinkerpop.gremlin.process.traversal.step.GValue; import org.apache.tinkerpop.gremlin.process.traversal.step.GValueHolder; import org.apache.tinkerpop.gremlin.process.traversal.step.util.AbstractStep; @@ -31,6 +33,7 @@ import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; import org.apache.tinkerpop.gremlin.structure.Element; import org.apache.tinkerpop.gremlin.structure.VertexProperty; +import java.util.ArrayList; import java.util.Collection; import java.util.Collections; import java.util.HashMap; @@ -50,7 +53,7 @@ public class AddPropertyStepPlaceholder<S extends Element> extends AbstractStep< /** * property value */ - private GValue<?> value; + private Traversal.Admin<?, ?> value; /** * cardinality of the property */ @@ -66,11 +69,19 @@ public class AddPropertyStepPlaceholder<S extends Element> extends AbstractStep< throw new IllegalArgumentException("GValue is not allowed for property keys"); } this.key = keyObject; - this.value = GValue.of(valueObject); - this.cardinality = cardinality; + if (this.key instanceof Traversal) { + this.integrateChild(((Traversal<?, ?>) this.key).asAdmin()); + } if (valueObject instanceof GValue) { traversal.getGValueManager().register((GValue<?>) valueObject); + this.value = new GValueConstantTraversal<>((GValue<?>) valueObject); + } else if (valueObject instanceof Traversal) { + this.value = ((Traversal<?, ?>) valueObject).asAdmin(); + this.integrateChild(((Traversal<?, ?>) valueObject).asAdmin()); + } else { + this.value = new ConstantTraversal<>(valueObject); } + this.cardinality = cardinality; } @Override @@ -79,8 +90,25 @@ public class AddPropertyStepPlaceholder<S extends Element> extends AbstractStep< } @Override - public <S, E> List<Traversal.Admin<S, E>> getLocalChildren() { - return Collections.emptyList(); //TODO:: is this right? + public List<Traversal.Admin<?, ?>> getLocalChildren() { + List<Traversal.Admin<?, ?>> childTraversals = new ArrayList<>(); + for (Map.Entry<Object, List<Object>> entry : properties.entrySet()) { + if (entry.getKey() instanceof Traversal) { + childTraversals.add((Traversal.Admin<?, ?>) ((Traversal) entry.getKey()).asAdmin()); + } + for (Object value : entry.getValue()) { + if (value instanceof Traversal) { + childTraversals.add((Traversal.Admin<?, ?>) ((Traversal) value).asAdmin()); + } + } + } + if (key != null && key instanceof Traversal) { + childTraversals.add(((Traversal<?, ?>) key).asAdmin()); + } + if (value != null) { + childTraversals.add(value); + } + return childTraversals; } @Override @@ -88,6 +116,12 @@ public class AddPropertyStepPlaceholder<S extends Element> extends AbstractStep< return this.getSelfAndChildRequirements(TraverserRequirement.OBJECT); } + @Override + public void setTraversal(final Traversal.Admin<?, ?> parentTraversal) { + super.setTraversal(parentTraversal); + this.getLocalChildren().forEach(this::integrateChild); + } + @Override public int hashCode() { return super.hashCode() ^ Objects.hashCode(key) ^ Objects.hashCode(value) ^ Objects.hashCode(cardinality); @@ -110,19 +144,28 @@ public class AddPropertyStepPlaceholder<S extends Element> extends AbstractStep< @Override public Object getValue() { - if (value != null) { - traversal.getGValueManager().pinVariable(value.getName()); - return value.get(); + if (value == null) { + return null; + } + if (value instanceof GValueConstantTraversal) { + traversal.getGValueManager().pinVariable(((GValueConstantTraversal<?, ?>) value).getGValue().getName()); + return value.next(); + } + if (value instanceof ConstantTraversal) { + return value.next(); } - return null; + return value; } /** * Get the value as a GValue, without pinning the variable */ @Override - public GValue<?> getValueAsGValue() { - return value; + public Object getValueWithGValue() { + if (value instanceof GValueConstantTraversal) { + return ((GValueConstantTraversal<?, ?>) value).getGValue(); + } + return getValue(); // Don't need to worry about pinning variable as GValue case is already covered } @Override @@ -137,7 +180,7 @@ public class AddPropertyStepPlaceholder<S extends Element> extends AbstractStep< @Override public AddPropertyStep<S> asConcreteStep() { - AddPropertyStep<S> step = new AddPropertyStep<>(traversal, cardinality, key, value.get()); + AddPropertyStep<S> step = new AddPropertyStep<>(traversal, cardinality, key, value instanceof GValueConstantTraversal ? ((GValueConstantTraversal<?, ?>) value).getConstantTraversal() : value); for (final Map.Entry<Object, List<Object>> entry : properties.entrySet()) { for (Object value : entry.getValue()) { @@ -151,7 +194,7 @@ public class AddPropertyStepPlaceholder<S extends Element> extends AbstractStep< @Override public boolean isParameterized() { - if (value.isVariable()) { + if (value instanceof GValueConstantTraversal && ((GValueConstantTraversal<?, ?>) value).isParameterized()) { return true; } for (List<Object> list : properties.values()) { @@ -164,8 +207,8 @@ public class AddPropertyStepPlaceholder<S extends Element> extends AbstractStep< @Override public void updateVariable(String name, Object value) { - if (name.equals(this.value.getName())) { - this.value = GValue.of(name, value); + if (value instanceof GValueConstantTraversal && name.equals(((GValueConstantTraversal<?, ?>) value).getGValue().getName())) { + this.value = new GValueConstantTraversal<>(GValue.of(name, value)); } for (final Map.Entry<Object, List<Object>> entry : properties.entrySet()) { for (final Object propertyVal : entry.getValue()) { @@ -179,8 +222,8 @@ public class AddPropertyStepPlaceholder<S extends Element> extends AbstractStep< @Override public Collection<GValue<?>> getGValues() { Set<GValue<?>> gValues = GValueHelper.getGValuesFromProperties(properties); - if (value.isVariable()) { - gValues.add(value); + if (value instanceof GValueConstantTraversal && ((GValueConstantTraversal<?, ?>) value).isParameterized()) { + gValues.add(((GValueConstantTraversal<?, ?>) value).getGValue()); } return gValues; } @@ -190,9 +233,15 @@ public class AddPropertyStepPlaceholder<S extends Element> extends AbstractStep< if (key instanceof GValue) { throw new IllegalArgumentException("GValue cannot be used as a property key"); } - if (value instanceof GValue) { //TODO could value come in as a traversal? + if (key instanceof Traversal) { + this.integrateChild(((Traversal<?, ?>) key).asAdmin()); + } + if (value instanceof GValue) { traversal.getGValueManager().register((GValue<?>) value); } + if (value instanceof Traversal) { + this.integrateChild(((Traversal<?, ?>) value).asAdmin()); + } if (properties.containsKey(key)) { throw new IllegalArgumentException("Only single value meta-properties are supported"); } diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParentTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParentTest.java new file mode 100644 index 0000000000..2117634e91 --- /dev/null +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/TraversalParentTest.java @@ -0,0 +1,1014 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.tinkerpop.gremlin.process.traversal.step; + +import java.time.OffsetDateTime; +import java.util.Arrays; +import java.util.List; +import java.util.Map; +import java.util.Set; + +import org.apache.commons.configuration2.MapConfiguration; +import org.apache.tinkerpop.gremlin.process.traversal.Operator; +import org.apache.tinkerpop.gremlin.process.traversal.Pick; +import org.apache.tinkerpop.gremlin.process.traversal.Step; +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.AbstractLambdaTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.ConstantTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.Merge; +import org.apache.tinkerpop.gremlin.process.traversal.Scope; +import org.apache.tinkerpop.gremlin.process.traversal.P; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.LoopTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.PredicateTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.TrueTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.lambda.ValueTraversal; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.AddEdgeStepContract; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.AddVertexStepContract; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.CallStepContract; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.MergeStepContract; +import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.AddPropertyStepContract; +import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.AggregateGlobalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.AggregateLocalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.AndStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.OrStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.branch.BranchStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.branch.ChooseStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.CoalesceStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.CombineStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.ConcatStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.DateDiffStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.DedupGlobalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.DifferenceStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.DisjunctStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.FormatStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.GroupCountStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.GroupCountSideEffectStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.GroupStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.IntersectStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.branch.LocalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.MatchStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.MathStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.MergeStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.NotStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.branch.OptionalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.OrderGlobalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.OrderLocalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.PathFilterStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.PathStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.ProductStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.ProjectStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.PropertyMapStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.branch.RepeatStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.SackValueStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.SampleGlobalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.SelectOneStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.SelectStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.TraversalSelectStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.TraversalFilterStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.TraversalFlatMapStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.TraversalMapStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.TraversalSideEffectStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.TreeStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.TreeSideEffectStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.branch.UnionStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.WherePredicateStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.WhereTraversalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.util.ComputerAwareStep; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.GValueReductionStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; +import org.apache.tinkerpop.gremlin.structure.T; +import org.apache.tinkerpop.gremlin.structure.service.Service; +import org.apache.tinkerpop.gremlin.structure.service.ServiceRegistry; + +import org.apache.tinkerpop.gremlin.structure.util.GraphFactoryTest; +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; + +import static org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource.traversal; +import static org.hamcrest.Matchers.containsInAnyOrder; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; + + +@RunWith(Parameterized.class) +public class TraversalParentTest { + + private static ServiceRegistry mockedRegistry= mock(ServiceRegistry.class); + private static Service<?, ?> mockedService = mock(Service.class); + private static GraphTraversalSource g = traversal().with(GraphFactoryTest.MockGraph.open(new MapConfiguration(Map.of("service-registry", mockedRegistry)))); + + @BeforeClass + public static void setupClass() { + when(mockedRegistry.get(any(), anyBoolean(), any())).thenReturn(mockedService); + when(mockedService.getRequirements()).thenReturn(Set.of()); + } + + @Parameterized.Parameter(value = 0) + public Class stepClass; + + @Parameterized.Parameter(value = 1) + public Traversal.Admin traversal; + + @Parameterized.Parameter(value = 2) + public List<Traversal.Admin<?,?>> expectedGlobalChildren; + + @Parameterized.Parameter(value = 3) + public List<Traversal.Admin<?,?>> expectedLocalChildren; + + /** + * Overrides expectedGlobalChildren for assertions following strategy execution. If left null, expectedGlobalChildren is used instead. + */ + @Parameterized.Parameter(value = 4) + public List<Traversal.Admin<?,?>> postStrategyExpectedGlobalChildren; + + /** + * Overrides expectedLocalChildren for assertions following strategy execution. If left null, expectedLocalChildren is used instead. + */ + @Parameterized.Parameter(value = 5) + public List<Traversal.Admin<?,?>> postStrategyExpectedLocalChildren; + + @Parameterized.Parameters(name = "{0}") + public static Iterable<Object[]> data() { + return Arrays.asList(new Object[][]{ + {AddVertexStepContract.class, + g.addV("label").property("name", __.constant("cole")), + List.of(), + List.of(__.constant("cole"), new ConstantTraversal<>("label")), + null, null + }, + {AddVertexStepContract.class, + g.addV(__.constant("label")).property("name", __.constant("cole")), + List.of(), + List.of(__.constant("cole"), __.constant("label")), + null, null + }, + {AddVertexStepContract.class, + g.addV(GValue.of("l", "label")).property("name", GValue.of("name", "cole")), + List.of(), + List.of(new ConstantTraversal<>("label")), // Property is not stored as a child traversal in this case + null, null + }, + {AddVertexStepContract.class, + g.addV("label").property(__.constant("name"), __.constant("cole")), + List.of(), + List.of(__.constant("name"), __.constant("cole"), new ConstantTraversal<>("label")), + null, null + }, + {AddVertexStepContract.class, + g.inject(1).addV("label").property("name", __.constant("cole")), + List.of(), + List.of(__.constant("cole"), new ConstantTraversal<>("label")), + null, null + }, + {AddVertexStepContract.class, + g.inject(1).addV(__.constant("label")).property("name", __.constant("cole")), + List.of(), + List.of(__.constant("cole"), __.constant("label")), + null, null + }, + {AddVertexStepContract.class, + g.inject(1).addV(GValue.of("l", "label")).property("name", GValue.of("name", "cole")), + List.of(), + List.of(new ConstantTraversal<>("label")), // Property is not stored as a child traversal in this case + null, null + }, + {AddVertexStepContract.class, + g.inject(1).addV("label").property(__.constant("name"), __.constant("cole")), + List.of(), + List.of(__.constant("name"), __.constant("cole"), new ConstantTraversal<>("label")), + null, null + }, + {AddEdgeStepContract.class, + g.addE("label").from(1).to(2).property("name", __.constant("cole")), + List.of(), + List.of(__.constant("cole"), new ConstantTraversal<>("label"), __.constant(1), __.constant(2)), + null, null + }, + {AddEdgeStepContract.class, + g.addE("label").from(__.V(1)).to(__.V(2)).property("name", __.constant("cole")), + List.of(), + List.of(__.constant("cole"), new ConstantTraversal<>("label"), __.V(1), __.V(2)), + null, null + }, + {AddEdgeStepContract.class, + g.addE(GValue.of("l", "label")).from(GValue.of("from", 1)).to(GValue.of("to", 2)).property("name", GValue.of("name", "cole")), + List.of(), + List.of(new ConstantTraversal<>("label"), new ConstantTraversal(1), new ConstantTraversal(2)), + null, null + }, + {AddEdgeStepContract.class, + g.addE("label").from(1).to(2).property(__.constant("name"), __.constant("cole")), + List.of(), + List.of(__.constant("name"), __.constant("cole"), new ConstantTraversal<>("label"), __.constant(1), __.constant(2)), + null, null + }, + {AddEdgeStepContract.class, + g.inject(1).addE("label").from(1).to(2).property("name", __.constant("cole")), + List.of(), + List.of(__.constant("cole"), new ConstantTraversal<>("label"), __.constant(1), __.constant(2)), + null, null + }, + {AddEdgeStepContract.class, + g.inject(1).addE("label").from(__.V(1)).to(__.V(2)).property("name", __.constant("cole")), + List.of(), + List.of(__.constant("cole"), new ConstantTraversal<>("label"), __.V(1), __.V(2)), + null, null + }, + {AddEdgeStepContract.class, + g.inject(1).addE(GValue.of("l", "label")).from(GValue.of("from", 1)).to(GValue.of("to", 2)).property("name", GValue.of("name", "cole")), + List.of(), + List.of(new ConstantTraversal<>("label"), new ConstantTraversal(1), new ConstantTraversal(2)), + null, null + }, + {AddEdgeStepContract.class, + g.inject(1).addE("label").from(1).to(2).property(__.constant("name"), __.constant("cole")), + List.of(), + List.of(__.constant("name"), __.constant("cole"), new ConstantTraversal<>("label"), __.constant(1), __.constant(2)), + null, null + }, + {CallStepContract.class, + g.call("service"), + List.of(), + List.of(), + null, null + }, + {CallStepContract.class, + g.call("service").with("key", __.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {CallStepContract.class, + g.call("service", __.constant(Map.of("key", "value"))), + List.of(), + List.of(__.constant(Map.of("key", "value"))), + null, null + }, + {CallStepContract.class, + g.call("service", __.constant(Map.of("key", "value"))).with("key", __.constant("value")), + List.of(), + List.of(__.constant(Map.of("key", "value")), __.constant("value")), + null, null + }, + {AddPropertyStepContract.class, + g.inject(1).property("key", "value"), + List.of(), + List.of(new ConstantTraversal<>("value")), // constant Value's are internally boxed in Traversals + null, null + }, + {AddPropertyStepContract.class, + g.inject(1).property("key", __.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {AddPropertyStepContract.class, + g.inject(1).property(__.constant("key"), __.constant("value")), + List.of(), + List.of(__.constant("key"), __.constant("value")), + null, null + }, + {AddPropertyStepContract.class, + g.addV().property("name", "value", "metaKey", "metaValue"), + List.of(), + List.of(new ConstantTraversal<>("value")), // constant Value's are internally boxed in Traversals + null, null + }, + {AddPropertyStepContract.class, + g.addV().property("name", __.constant("value"), "metaKey", __.constant("metaValue")), + List.of(), + List.of(__.constant("value"), __.constant("metaValue")), + null, null + }, + {AddPropertyStepContract.class, + g.addV().property(__.constant("name"), __.constant("value"), __.constant("metaKey"), __.constant("metaValue")), + List.of(), + List.of(__.constant("name"), __.constant("value"), __.constant("metaKey"), __.constant("metaValue")), + null, null + }, + {AddPropertyStepContract.class, + g.addV().property("name", "value", "metaKey1", "metaValue1", "metaKey2", "metaValue2"), + List.of(), + List.of(new ConstantTraversal<>("value")), // constant Value's are internally boxed in Traversals + null, null + }, + {MergeStepContract.class, + g.mergeV(Map.of("name", "marko")), + List.of(), + List.of(new ConstantTraversal<>(Map.of("name", "marko"))), // constant MergeMap's are internally boxed in Traversals + null, null + }, + {MergeStepContract.class, + g.mergeV(__.constant(Map.of("name", "marko"))), + List.of(), + List.of(__.constant(Map.of("name", "marko"))), + null, null + }, + {MergeStepContract.class, + g.mergeV(Map.of("name", "marko")).option(Merge.onCreate, __.constant(Map.of("age", 29))), + List.of(), + List.of(new ConstantTraversal<>(Map.of("name", "marko")), __.constant(Map.of("age", 29))), // constant MergeMap's are internally boxed in Traversals + null, null + }, + {MergeStepContract.class, + g.mergeV(__.constant(Map.of("name", "marko"))).option(Merge.onCreate, __.constant(Map.of("age", 29))).option(Merge.onMatch, __.constant(Map.of("updated", true))), + List.of(), + List.of(__.constant(Map.of("name", "marko")), __.constant(Map.of("age", 29)), __.constant(Map.of("updated", true))), + null, null + }, + {MergeStepContract.class, + g.mergeE(Map.of(T.label, "knows")), + List.of(), + List.of(new ConstantTraversal<>(Map.of(T.label, "knows"))), // constant MergeMap's are internally boxed in Traversals + null, null + }, + {MergeStepContract.class, + g.mergeE(__.constant(Map.of(T.label, "knows"))), + List.of(), + List.of(__.constant(Map.of(T.label, "knows"))), + null, null + }, + {MergeStepContract.class, + g.mergeE(Map.of(T.label, "knows")).option(Merge.onCreate, __.constant(Map.of("weight", 0.5))), + List.of(), + List.of(new ConstantTraversal<>(Map.of(T.label, "knows")), __.constant(Map.of("weight", 0.5))), // constant MergeMap's are internally boxed in Traversals + null, null + }, + {MergeStepContract.class, + g.mergeE(__.constant(Map.of(T.label, "knows"))).option(Merge.onCreate, __.constant(Map.of("weight", 0.5))).option(Merge.onMatch, __.constant(Map.of("updated", true))), + List.of(), + List.of(__.constant(Map.of(T.label, "knows")), __.constant(Map.of("weight", 0.5)), __.constant(Map.of("updated", true))), + null, null + }, + {AggregateGlobalStep.class, + g.V().aggregate("x"), + List.of(), + List.of(), + null, null + }, + {AggregateGlobalStep.class, + g.V().aggregate("x").by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {AggregateGlobalStep.class, + g.V().aggregate("x").by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {AggregateLocalStep.class, + g.V().aggregate(Scope.local, "x"), + List.of(), + List.of(), + null, null + }, + {AggregateLocalStep.class, + g.V().aggregate(Scope.local, "x").by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {AggregateLocalStep.class, + g.V().aggregate(Scope.local, "x").by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {AndStep.class, + g.V().and(__.outE(), __.has("age", P.gte(32))), + List.of(), + List.of(__.outE(), __.has("age", P.gte(32))), + null, null + }, + {OrStep.class, + g.V().or(__.outE(), __.has("age", P.gte(32))), + List.of(), + List.of(__.outE(), __.has("age", P.gte(32))), + null, null + }, + {BranchStep.class, + g.V().branch(__.label()).option("person", __.values("name")).option("software", __.values("lang")), + List.of(appendComputerAwareEndStep(__.values("name")), appendComputerAwareEndStep(__.values("lang"))), + List.of(__.label(), new PredicateTraversal<>(P.eq("person")), new PredicateTraversal<>(P.eq("software"))), // option keys are wrapped in PredicateTraversals internally + null, null + }, + {ChooseStep.class, + g.V().choose(__.out().count()).option(2L, __.values("name")).option(3L, __.values("age")), + List.of(appendComputerAwareEndStep(__.values("name")), appendComputerAwareEndStep(__.values("age")), appendComputerAwareEndStep(__.identity()), appendComputerAwareEndStep(__.identity())), + List.of(__.out().count(), new PredicateTraversal<>(P.eq(2L)), new PredicateTraversal<>(P.eq(3L))), // option keys are wrapped in PredicateTraversals internally + null, List.of(__.outE().count(), new PredicateTraversal<>(P.eq(2L)), new PredicateTraversal<>(P.eq(3L))) + }, + {CoalesceStep.class, + g.V().coalesce(__.out("knows"), __.out("created")), + List.of(), + List.of(__.out("knows"), __.out("created")), + null, null + }, + {CombineStep.class, + g.V().fold().combine(__.constant(List.of(1, 2, 3))), + List.of(), + List.of(__.constant(List.of(1, 2, 3))), + null, null + }, + {ConcatStep.class, + g.inject("a", "b").concat(__.constant("c"), __.constant("d")), + List.of(), + List.of(__.constant("c"), __.constant("d")), + null, null + }, + {DateDiffStep.class, + g.inject(OffsetDateTime.parse("1970-01-01T00:00:00.000Z")).dateDiff(__.constant(OffsetDateTime.parse("1970-02-01T00:00:00.000Z"))), + List.of(), + List.of(__.constant(OffsetDateTime.parse("1970-02-01T00:00:00.000Z"))), + null, null + }, + {DedupGlobalStep.class, + g.V().dedup(), + List.of(), + List.of(), + null, null + }, + {DedupGlobalStep.class, + g.V().dedup().by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {DedupGlobalStep.class, + g.V().dedup().by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {DifferenceStep.class, + g.V().fold().difference(__.constant(List.of(1, 2, 3))), + List.of(), + List.of(__.constant(List.of(1, 2, 3))), + null, null + }, + {DisjunctStep.class, + g.V().fold().disjunct(__.constant(List.of(1, 2, 3))), + List.of(), + List.of(__.constant(List.of(1, 2, 3))), + null, null + }, + {FormatStep.class, + g.V().format("Hello %{name}"), + List.of(), + List.of(), + null, null + }, + {FormatStep.class, + g.V().format("Hello %{name}").by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {FormatStep.class, + g.V().format("Hello %{name}").by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {GroupCountStep.class, + g.V().groupCount(), + List.of(), + List.of(), + null, null + }, + {GroupCountStep.class, + g.V().groupCount().by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {GroupCountStep.class, + g.V().groupCount().by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {GroupCountSideEffectStep.class, + g.V().groupCount("x"), + List.of(), + List.of(), + null, null + }, + {GroupCountSideEffectStep.class, + g.V().groupCount("x").by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {GroupCountSideEffectStep.class, + g.V().groupCount("x").by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {GroupStep.class, + g.V().group(), + List.of(), + List.of(__.fold()), + null, null + }, + {GroupStep.class, + g.V().group().by("name"), + List.of(), + List.of(new ValueTraversal("name"), __.fold()), + null, null + }, + {GroupStep.class, + g.V().group().by("name").by("age"), + List.of(), + List.of(new ValueTraversal("name"), __.map(new ValueTraversal("age")).fold()), + null, null + }, + {GroupStep.class, + g.V().group().by(__.constant("key")).by(__.constant("value")), + List.of(), + List.of(__.constant("key"), __.constant("value")), + null, null + }, + {IntersectStep.class, + g.V().fold().intersect(__.constant(List.of(1, 2, 3))), + List.of(), + List.of(__.constant(List.of(1, 2, 3))), + null, null + }, + {LocalStep.class, + g.V().local(__.outE().count()), + List.of(), + List.of(__.outE().count()), + null, null + }, + {MatchStep.class, + g.V().match(__.as("a").out().as("b"), __.as("b").in().as("c")), + List.of(generateMatchChildTraversal("a", "b", __.out().asAdmin()), generateMatchChildTraversal("b", "c", __.in().asAdmin())), + List.of(), + null, null + }, + {MathStep.class, + g.V().math("_ + 1"), + List.of(), + List.of(), + null, null + }, + {MathStep.class, + g.V().math("_ + 1").by("age"), + List.of(), + List.of(new ValueTraversal("age")), + null, null + }, + {MathStep.class, + g.V().math("_ + _").by("age").by(__.constant(10)), + List.of(), + List.of(new ValueTraversal("age"), __.constant(10)), + null, null + }, + {MergeStep.class, + g.V().fold().merge(__.constant(List.of(1, 2, 3))), + List.of(), + List.of(__.constant(List.of(1, 2, 3))), + null, null + }, + {NotStep.class, + g.V().not(__.has("age", P.gt(27))), + List.of(), + List.of(__.has("age", P.gt(27))), + null, null + }, + {OptionalStep.class, + g.V().optional(__.out("knows")), + List.of(), + List.of(__.out("knows")), + null, null + }, + {OrderGlobalStep.class, + g.V().order(), + List.of(), + List.of(), + null, null + }, + {OrderGlobalStep.class, + g.V().order().by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {OrderGlobalStep.class, + g.V().order().by("name").by(__.constant("value")), + List.of(), + List.of(new ValueTraversal("name"), __.constant("value")), + null, null + }, + {OrderLocalStep.class, + g.V().fold().order(Scope.local), + List.of(), + List.of(), + null, null + }, + {OrderLocalStep.class, + g.V().fold().order(Scope.local).by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {OrderLocalStep.class, + g.V().fold().order(Scope.local).by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {PathFilterStep.class, + g.V().simplePath(), + List.of(), + List.of(), + null, null + }, + {PathFilterStep.class, + g.V().cyclicPath(), + List.of(), + List.of(), + null, null + }, + {PathFilterStep.class, + g.V().simplePath().by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {PathFilterStep.class, + g.V().simplePath().by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {PathFilterStep.class, + g.V().cyclicPath().by("age").by(__.constant("value")), + List.of(), + List.of(new ValueTraversal("age"), __.constant("value")), + null, null + }, + {PathStep.class, + g.V().path(), + List.of(), + List.of(), + null, null + }, + {PathStep.class, + g.V().out().path().by("age").by("name"), + List.of(), + List.of(new ValueTraversal("age"), new ValueTraversal("name")), + null, null + }, + {PathStep.class, + g.V().path().by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {ProductStep.class, + g.V().fold().product(__.constant(List.of(1, 2, 3))), + List.of(), + List.of(__.constant(List.of(1, 2, 3))), + null, null + }, + {ProjectStep.class, + g.V().project("a", "b").by("name").by("age"), + List.of(), + List.of(new ValueTraversal("name"), new ValueTraversal("age")), + null, null + }, + {ProjectStep.class, + g.V().project("x").by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {PropertyMapStep.class, + g.V().valueMap(), + List.of(), + List.of(), + null, null + }, + {PropertyMapStep.class, + g.V().valueMap().by(__.constant("transformed")), + List.of(), + List.of(__.constant("transformed")), + null, null + }, + {RepeatStep.class, + g.V().repeat(__.out()).emit().times(2), + List.of(appendRepeatEndStep(__.out())), + List.of(new LoopTraversal<>(2), TrueTraversal.instance()), + null, null + }, + {RepeatStep.class, + g.V().repeat(__.out()).until(__.has("name", "josh")), + List.of(appendRepeatEndStep(__.out())), + List.of(__.has("name", "josh")), + null, null + }, + {RepeatStep.class, + g.V().repeat(__.out()).emit(__.has("age")).until(__.has("name", "josh")), + List.of(appendRepeatEndStep(__.out())), + List.of(__.has("name", "josh"), __.has("age")), + null, List.of(__.has("name", "josh"), __.filter(__.properties("age"))), + }, + {SackValueStep.class, + g.withSack(0).V().sack(Operator.sum), + List.of(), + List.of(), + null, null + }, + {SackValueStep.class, + g.withSack(0).V().sack(Operator.sum).by("age"), + List.of(), + List.of(new ValueTraversal("age")), + null, null + }, + {SackValueStep.class, + g.withSack(0).V().sack(Operator.sum).by(__.constant("age")), + List.of(), + List.of(__.constant("age")), + null, null + }, + {SampleGlobalStep.class, + g.V().sample(2), + List.of(), + List.of(new ConstantTraversal<>(1.0)), + null, null + }, + {SampleGlobalStep.class, + g.E().sample(2).by("weight"), + List.of(), + List.of(new ValueTraversal("weight")), + null, null + }, + {SampleGlobalStep.class, + g.E().sample(2).by(__.constant("weight")), + List.of(), + List.of(__.constant("weight")), + null, null + }, + {SelectOneStep.class, + g.V().as("a").select("a"), + List.of(), + List.of(), + null, null + }, + {SelectOneStep.class, + g.V().as("a").select("a").by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {SelectOneStep.class, + g.V().as("a").select("a").by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {SelectStep.class, + g.V().as("a").as("b").select("a", "b"), + List.of(), + List.of(), + null, null + }, + {SelectStep.class, + g.V().as("a").as("b").select("a", "b").by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {SelectStep.class, + g.V().as("a").as("b").select("a", "b").by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {TraversalSelectStep.class, + g.V().select((Traversal)__.constant("a")), + List.of(), + List.of(__.constant("a")), + null, null + }, + {TraversalSelectStep.class, + g.V().select((Traversal)__.constant("a")).by("name"), + List.of(), + List.of(__.constant("a"), new ValueTraversal("name")), + null, null + }, + {TraversalFilterStep.class, + g.V().filter(__.constant(5)), // a bit of an arbitrary case to avoid getting optimized by strategies + List.of(), + List.of(__.constant(5)), + null, null + }, + {TraversalFlatMapStep.class, + g.V().flatMap(__.out()), + List.of(), + List.of(__.out()), + null, null + }, + {TraversalFlatMapStep.class, + g.V().flatMap(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {TraversalMapStep.class, + g.V().map(__.values("name")), + List.of(), + List.of(__.values("name")), + null, null + }, + {TraversalMapStep.class, + g.V().map(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {TraversalSideEffectStep.class, + g.V().sideEffect(__.identity()), + List.of(), + List.of(__.identity()), + null, null + }, + {TraversalSideEffectStep.class, + g.V().sideEffect(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {TreeStep.class, + g.V().out().tree(), + List.of(), + List.of(), + null, null + }, + {TreeStep.class, + g.V().out().tree().by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {TreeStep.class, + g.V().out().tree().by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {TreeSideEffectStep.class, + g.V().out().tree("x"), + List.of(), + List.of(), + null, null + }, + {TreeSideEffectStep.class, + g.V().out().tree("x").by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {TreeSideEffectStep.class, + g.V().out().tree("x").by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {UnionStep.class, + g.union(__.V().values("name"), __.V().values("age")), + List.of(appendComputerAwareEndStep(__.V().values("name")), appendComputerAwareEndStep(__.V().values("age"))), + List.of(new ConstantTraversal<>(Pick.any)), + List.of(appendComputerAwareEndStep(__.V().values("name").barrier(2500)), appendComputerAwareEndStep(__.V().values("age").barrier(2500))), + null + }, + {UnionStep.class, + g.union(__.constant("a"), __.constant("b")), + List.of(appendComputerAwareEndStep(__.constant("a")), appendComputerAwareEndStep(__.constant("b"))), + List.of(new ConstantTraversal<>(Pick.any)), + null, null + }, + {WherePredicateStep.class, + g.V().as("a").out().as("b").where("a", P.eq("b")), + List.of(), + List.of(), + null, null + }, + {WherePredicateStep.class, + g.V().as("a").out().as("b").where("a", P.eq("b")).by("name"), + List.of(), + List.of(new ValueTraversal("name")), + null, null + }, + {WherePredicateStep.class, + g.V().as("a").out().as("b").where("a", P.eq("b")).by(__.constant("value")), + List.of(), + List.of(__.constant("value")), + null, null + }, + {WhereTraversalStep.class, + g.V().as("a").out().as("b").where(__.as("a").out().as("c")), + List.of(), + List.of(generateWhereChildTraversal("a", "c", __.out().asAdmin())), + null, null + }, + }); + } + + @Test + public void doTest() { + verifyExpectedParents(expectedGlobalChildren, expectedLocalChildren, "(pre-strategy application)"); + + traversal = traversal.clone(); + + verifyExpectedParents(expectedGlobalChildren, expectedLocalChildren, "(cloned traversal, pre-strategy application)"); + + traversal.getStrategies().addStrategies(GValueReductionStrategy.instance()); + traversal.applyStrategies(); + + verifyExpectedParents(postStrategyExpectedGlobalChildren == null ? expectedGlobalChildren : postStrategyExpectedGlobalChildren, + postStrategyExpectedLocalChildren == null ? expectedLocalChildren : postStrategyExpectedLocalChildren, + "(pre-strategy application)"); + + traversal = traversal.clone(); + + verifyExpectedParents(postStrategyExpectedGlobalChildren == null ? expectedGlobalChildren : postStrategyExpectedGlobalChildren, + postStrategyExpectedLocalChildren == null ? expectedLocalChildren : postStrategyExpectedLocalChildren, + "(cloned traversal, pre-strategy application)"); + } + + private void verifyExpectedParents(List<Traversal.Admin<?,?>> expectedGlobalChildren, List<Traversal.Admin<?,?>> expectedLocalChildren, String messageSuffix) { + List<Step> steps = TraversalHelper.getStepsOfAssignableClass(stepClass, traversal); + assertTrue(String.format("Expected a single step of class %s to test, found %d %s", stepClass.getName(), steps.size(), messageSuffix), steps.size() == 1); + assertTrue(String.format("Expected step to implement TraversalParent %s", messageSuffix), steps.get(0) instanceof TraversalParent); + + TraversalParent testStep = (TraversalParent) steps.get(0); + assertThat(String.format("getGlobalChildren() did not produce the expected results %s", messageSuffix), testStep.getGlobalChildren(), containsInAnyOrder(expectedGlobalChildren.toArray())); + assertThat(String.format("getLocalChildren() did not produce the expected results %s", messageSuffix), testStep.getLocalChildren(), containsInAnyOrder(expectedLocalChildren.toArray())); + + for (Traversal.Admin<?, ?> child : testStep.getGlobalChildren()) { + if (!(child instanceof AbstractLambdaTraversal)) { + // Use reference equality to ensure parent points to the correct object. It's not enough that the parents contents match. + assertTrue(String.format("Expected child traversal %s to have correctly assigned parent %s", child.toString(), messageSuffix), testStep == child.getParent()); + } + } + for (Traversal.Admin<?, ?> child : testStep.getLocalChildren()) { + if (!(child instanceof AbstractLambdaTraversal)) { + // Use reference equality to ensure parent points to the correct object. It's not enough that the parents contents match. + assertTrue(String.format("Expected child traversal %s to have correctly assigned parent %s", child.toString(), messageSuffix), testStep == child.getParent()); + } + } + } + + private static Traversal<?, ?> appendComputerAwareEndStep(final Traversal<?, ?> traversal) { + traversal.asAdmin().addStep(new ComputerAwareStep.EndStep(traversal.asAdmin())); + return traversal; + } + + private static Traversal<?, ?> appendRepeatEndStep(final Traversal<?, ?> traversal) { + traversal.asAdmin().addStep(new RepeatStep.RepeatEndStep<>(traversal.asAdmin())); + return traversal; + } + + private static Traversal<?, ?> generateMatchChildTraversal(final String start, final String end, final Traversal.Admin<?,?> traversal) { + traversal.addStep(0, new MatchStep.MatchStartStep(traversal, start)); + traversal.addStep(new MatchStep.MatchEndStep(traversal, end)); + return traversal; + } + + private static Traversal<?, ?> generateWhereChildTraversal(final String start, final String end, final Traversal.Admin<?,?> traversal) { + traversal.addStep(0, new WhereTraversalStep.WhereStartStep<>(traversal, start)); + traversal.addStep(new WhereTraversalStep.WhereEndStep(traversal, end)); + return traversal; + } +} diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepTest.java index c27fa13ea8..9860e99bf7 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/sideEffect/AddPropertyStepTest.java @@ -99,7 +99,7 @@ public class AddPropertyStepTest extends GValueStepTest { @Test public void getValueAsGValueShouldNotPinVariable() { final GraphTraversal.Admin<Object, Object> traversal = __.property(PNAME, GValue.of(GNAME, PVALUE)).asAdmin(); - assertEquals(GValue.of(GNAME, PVALUE), ((AddPropertyStepPlaceholder) traversal.getSteps().get(0)).getValueAsGValue()); + assertEquals(GValue.of(GNAME, PVALUE), ((AddPropertyStepPlaceholder) traversal.getSteps().get(0)).getValueWithGValue()); verifySingleUnpinnedVariable(traversal, GNAME); }
