This is an automated email from the ASF dual-hosted git repository. xiazcy pushed a commit to branch steps-taking-traversal-poc in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit eee4d8192bc0a45f574f30a820011f23b94ff8cc Author: Yang Xia <[email protected]> AuthorDate: Thu Jun 4 17:12:35 2026 -0700 clean up & refactors --- .../tinkerpop/gremlin/process/traversal/P.java | 48 ++++++++------- .../tinkerpop/gremlin/process/traversal/TextP.java | 12 ++-- .../traversal/dsl/graph/GraphTraversal.java | 14 ++--- .../traversal/dsl/graph/GraphTraversalSource.java | 4 +- .../traversal/step/filter/WherePredicateStep.java | 58 +++++++++--------- .../ChildTraversalVerificationStrategy.java | 44 +++++--------- .../traversal/util/ChildTraversalValidator.java | 69 +++++----------------- 7 files changed, 94 insertions(+), 155 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java index 72cd3bcc5d..f95001a55b 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java @@ -346,27 +346,26 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { trav.reset(); trav.addStart(split); - this.isCollection = false; - try { - // All predicates take only the first result — consistent with by(traversal). - // For within/without, the first result should be a Collection (e.g., from fold()). + // Take first result only — consistent with by(traversal) semantics. if (!trav.hasNext()) { this.resolvedEmpty = true; this.literals = Collections.emptyList(); + this.isCollection = false; } else { this.resolvedEmpty = false; final Object firstResult = trav.next(); - if (this.biPredicate instanceof Contains && firstResult instanceof Collection) { - // The traversal produced a Collection as its first result — use it directly - this.literals = (Collection<V>) firstResult; - this.isCollection = true; - } else if (this.biPredicate instanceof Contains) { - // Single non-Collection value for within/without — wrap in a singleton list - this.literals = Collections.singletonList((V) firstResult); + if (this.biPredicate instanceof Contains) { + // Contains predicates need a Collection value. If first result is already + // a Collection (e.g., from fold()), use it directly. Otherwise wrap in singleton. + if (firstResult instanceof Collection) { + this.literals = (Collection<V>) firstResult; + } else { + this.literals = Collections.singletonList((V) firstResult); + } this.isCollection = true; } else { - this.literals = Collections.singleton((V) firstResult); + setValue((V) firstResult); } } } finally { @@ -391,8 +390,8 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { trav.addStart(split); try { - // Take only the first result from each traversal — consistent with by(traversal). - // If the first result is a Collection (from fold()), unpack it into the results. + // Take first result only from each traversal. + // If the result is a Collection (from fold()), unpack it. if (trav.hasNext()) { final Object firstResult = trav.next(); if (firstResult instanceof Collection) { @@ -407,7 +406,6 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { } this.resolvedEmpty = allResults.isEmpty(); - if (allResults.isEmpty()) { this.literals = Collections.emptyList(); this.isCollection = false; @@ -730,7 +728,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { * @since 4.0.0 */ public static <V> P<V> eq(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new P(Compare.eq, traversalValue.asAdmin()); } @@ -740,7 +738,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { * @since 4.0.0 */ public static <V> P<V> neq(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new P(Compare.neq, traversalValue.asAdmin()); } @@ -750,7 +748,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { * @since 4.0.0 */ public static <V> P<V> lt(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new P(Compare.lt, traversalValue.asAdmin()); } @@ -760,7 +758,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { * @since 4.0.0 */ public static <V> P<V> lte(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new P(Compare.lte, traversalValue.asAdmin()); } @@ -770,7 +768,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { * @since 4.0.0 */ public static <V> P<V> gt(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new P(Compare.gt, traversalValue.asAdmin()); } @@ -780,7 +778,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { * @since 4.0.0 */ public static <V> P<V> gte(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new P(Compare.gte, traversalValue.asAdmin()); } @@ -790,7 +788,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { * @since 4.0.0 */ public static <V> P<V> within(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new P(Contains.within, traversalValue.asAdmin()); } @@ -810,7 +808,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { } } for (final Traversal.Admin<?, ?> tv : traversals) { - ChildTraversalValidator.validateFilterContext(tv); + ChildTraversalValidator.validate(tv); } return new P(Contains.within, traversals); } @@ -821,7 +819,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { * @since 4.0.0 */ public static <V> P<V> without(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new P(Contains.without, traversalValue.asAdmin()); } @@ -841,7 +839,7 @@ public class P<V> implements Predicate<V>, Serializable, Cloneable { } } for (final Traversal.Admin<?, ?> tv : traversals) { - ChildTraversalValidator.validateFilterContext(tv); + ChildTraversalValidator.validate(tv); } return new P(Contains.without, traversals); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TextP.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TextP.java index dcbe70a0cd..c0bd155185 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TextP.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TextP.java @@ -93,7 +93,7 @@ public class TextP extends P<String> { * @since 4.0.0 */ public static TextP startingWith(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new TextP(Text.startingWith, traversalValue.asAdmin()); } @@ -121,7 +121,7 @@ public class TextP extends P<String> { * @since 4.0.0 */ public static TextP notStartingWith(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new TextP(Text.notStartingWith, traversalValue.asAdmin()); } @@ -149,7 +149,7 @@ public class TextP extends P<String> { * @since 4.0.0 */ public static TextP endingWith(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new TextP(Text.endingWith, traversalValue.asAdmin()); } @@ -177,7 +177,7 @@ public class TextP extends P<String> { * @since 4.0.0 */ public static TextP notEndingWith(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new TextP(Text.notEndingWith, traversalValue.asAdmin()); } @@ -205,7 +205,7 @@ public class TextP extends P<String> { * @since 4.0.0 */ public static TextP containing(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new TextP(Text.containing, traversalValue.asAdmin()); } @@ -233,7 +233,7 @@ public class TextP extends P<String> { * @since 4.0.0 */ public static TextP notContaining(final Traversal<?, ?> traversalValue) { - ChildTraversalValidator.validateFilterContext(traversalValue.asAdmin()); + ChildTraversalValidator.validate(traversalValue.asAdmin()); return new TextP(Text.notContaining, traversalValue.asAdmin()); } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java index 552dec3be8..e015e708cb 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversal.java @@ -427,7 +427,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { */ public default GraphTraversal<S, Vertex> V(final Traversal<?, ?> traversal) { if (null == traversal) return V(new Object[]{ null }); - ChildTraversalValidator.validateLookupContext(traversal.asAdmin()); + ChildTraversalValidator.validate(traversal.asAdmin()); this.asAdmin().getGremlinLang().addStep(Symbols.V, traversal); return this.asAdmin().addStep(new GraphStep<>(this.asAdmin(), Vertex.class, false, traversal.asAdmin())); } @@ -467,7 +467,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { */ public default GraphTraversal<S, Edge> E(final Traversal<?, ?> traversal) { if (null == traversal) return E(new Object[]{ null }); - ChildTraversalValidator.validateLookupContext(traversal.asAdmin()); + ChildTraversalValidator.validate(traversal.asAdmin()); this.asAdmin().getGremlinLang().addStep(Symbols.E, traversal); return this.asAdmin().addStep(new GraphStep<>(this.asAdmin(), Edge.class, false, traversal.asAdmin())); } @@ -2718,7 +2718,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { */ public default GraphTraversal<S, E> has(final String propertyKey, final Traversal<?, ?> traversal) { if (null == traversal) return has(propertyKey, (Object) null); - ChildTraversalValidator.validateFilterContext(traversal.asAdmin()); + ChildTraversalValidator.validate(traversal.asAdmin()); this.asAdmin().getGremlinLang().addStep(Symbols.has, propertyKey, traversal); final HasContainer hasContainer = new HasContainer(propertyKey, traversal.asAdmin()); return this.asAdmin().addStep(new HasStep(this.asAdmin(), hasContainer)); @@ -2739,7 +2739,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { if (null == accessor) throw new IllegalArgumentException("The T accessor value of has(T,Traversal) cannot be null"); if (null == traversal) return has(accessor, (Object) null); - ChildTraversalValidator.validateFilterContext(traversal.asAdmin()); + ChildTraversalValidator.validate(traversal.asAdmin()); this.asAdmin().getGremlinLang().addStep(Symbols.has, accessor, traversal); final HasContainer hasContainer = new HasContainer(accessor.getAccessor(), traversal.asAdmin()); @@ -2792,7 +2792,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { */ public default GraphTraversal<S, E> has(final String label, final String propertyKey, final Traversal<?, ?> traversal) { if (null == traversal) return has(label, propertyKey, (Object) null); - ChildTraversalValidator.validateFilterContext(traversal.asAdmin()); + ChildTraversalValidator.validate(traversal.asAdmin()); this.asAdmin().getGremlinLang().addStep(Symbols.has, label, propertyKey, traversal); TraversalHelper.addHasContainer(this.asAdmin(), new HasContainer(T.label.getAccessor(), P.eq(label))); final HasContainer hasContainer = new HasContainer(propertyKey, traversal.asAdmin()); @@ -2905,7 +2905,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { */ public default GraphTraversal<S, E> hasLabel(final Traversal<?, ?> traversal) { if (null == traversal) return hasLabel((String) null); - ChildTraversalValidator.validateFilterContext(traversal.asAdmin()); + ChildTraversalValidator.validate(traversal.asAdmin()); this.asAdmin().getGremlinLang().addStep(Symbols.hasLabel, traversal); final HasContainer hasContainer = new HasContainer(T.label.getAccessor(), traversal.asAdmin()); return this.asAdmin().addStep(new HasStep(this.asAdmin(), hasContainer)); @@ -3959,7 +3959,7 @@ public interface GraphTraversal<S, E> extends Traversal<S, E> { */ public default GraphTraversal<S, E> property(final Traversal<?, ?> mapTraversal) { if (null == mapTraversal) return this; - ChildTraversalValidator.validateMutationContext(mapTraversal.asAdmin()); + ChildTraversalValidator.validate(mapTraversal.asAdmin()); this.asAdmin().getGremlinLang().addStep(Symbols.property, mapTraversal); this.asAdmin().addStep(new AddPropertyStepPlaceholder(this.asAdmin(), null, "~traversalMap", mapTraversal.asAdmin())); return this; diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java index 96a9ae7980..bf33e619c5 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/dsl/graph/GraphTraversalSource.java @@ -549,7 +549,7 @@ public class GraphTraversalSource implements TraversalSource { */ public GraphTraversal<Vertex, Vertex> V(final Traversal<?, ?> traversal) { if (null == traversal) return V(new Object[]{ null }); - ChildTraversalValidator.validateLookupContext(traversal.asAdmin()); + ChildTraversalValidator.validate(traversal.asAdmin()); final GraphTraversalSource clone = this.clone(); clone.gremlinLang.addStep(GraphTraversal.Symbols.V, traversal); final GraphTraversal.Admin<Vertex, Vertex> traversalAdmin = new DefaultGraphTraversal<>(clone); @@ -588,7 +588,7 @@ public class GraphTraversalSource implements TraversalSource { */ public GraphTraversal<Edge, Edge> E(final Traversal<?, ?> traversal) { if (null == traversal) return E(new Object[]{ null }); - ChildTraversalValidator.validateLookupContext(traversal.asAdmin()); + ChildTraversalValidator.validate(traversal.asAdmin()); final GraphTraversalSource clone = this.clone(); clone.gremlinLang.addStep(GraphTraversal.Symbols.E, traversal); final GraphTraversal.Admin<Edge, Edge> traversalAdmin = new DefaultGraphTraversal<>(clone); diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WherePredicateStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WherePredicateStep.java index ce47519ea7..a3bd10ea48 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WherePredicateStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/WherePredicateStep.java @@ -54,6 +54,7 @@ public final class WherePredicateStep<S> extends FilterStep<S> implements Scopin protected Set<String> keepLabels; protected TraversalRing<S, ?> traversalRing = new TraversalRing<>(); + private List<Traversal.Admin<?, ?>> predicateTraversals; public WherePredicateStep(final Traversal.Admin traversal, final Optional<String> startKey, final P<String> predicate) { super(traversal); @@ -63,8 +64,12 @@ public final class WherePredicateStep<S> extends FilterStep<S> implements Scopin this.predicate = (P) predicate; this.selectKeys = new ArrayList<>(); this.configurePredicates(this.predicate); - // Integrate child traversals from the predicate so they participate in cloning/strategy application - P.integrateTraversals(this.predicate, this); + // Cache and integrate child traversals from the predicate + this.predicateTraversals = new ArrayList<>(); + P.collectTraversals(this.predicate, this.predicateTraversals); + for (final Traversal.Admin<?, ?> t : this.predicateTraversals) { + this.integrateChild(t); + } } private void configurePredicates(final P<Object> predicate) { @@ -115,35 +120,28 @@ public final class WherePredicateStep<S> extends FilterStep<S> implements Scopin @Override protected boolean filter(final Traverser.Admin<S> traverser) { - // If the predicate has a child traversal, resolve it and test directly against the - // current value (or startKey scope value). This bypasses the scope-label mechanism. - if (this.predicate.hasTraversal()) { - final Object leftValue; - if (null == this.startKey) { - final TraversalProduct product = TraversalUtil.produce(traverser, this.traversalRing.next()); - this.traversalRing.reset(); - if (!product.isProductive()) return false; - leftValue = product.get(); - } else { - final TraversalProduct product = TraversalUtil.produce((S) this.getSafeScopeValue(Pop.last, this.startKey, traverser), this.traversalRing.next()); - this.traversalRing.reset(); - if (!product.isProductive()) return false; - leftValue = product.get(); - } - this.predicate.resolve(traverser); - if (this.predicate.isResolvedEmpty()) return false; - return this.predicate.test(leftValue); - } - - // Standard scope-label path for non-traversal predicates + // Resolve the left-hand value (current traverser or startKey scope value) final TraversalProduct product = null == this.startKey ? TraversalUtil.produce(traverser, this.traversalRing.next()) : TraversalUtil.produce((S) this.getSafeScopeValue(Pop.last, this.startKey, traverser), this.traversalRing.next()); - final boolean predicateValuesProductive = this.setPredicateValues(this.predicate, traverser, this.selectKeys.iterator()); - this.traversalRing.reset(); + if (!product.isProductive()) { + this.traversalRing.reset(); + return false; + } - return product.isProductive() && predicateValuesProductive && this.predicate.test(product.get()); + if (this.predicate.hasTraversal()) { + // Traversal-bearing predicate: resolve the child traversal, then test + this.traversalRing.reset(); + this.predicate.resolve(traverser); + if (this.predicate.isResolvedEmpty()) return false; + return this.predicate.test(product.get()); + } else { + // Standard scope-label path: resolve predicate values from path labels + final boolean predicateValuesProductive = this.setPredicateValues(this.predicate, traverser, this.selectKeys.iterator()); + this.traversalRing.reset(); + return predicateValuesProductive && this.predicate.test(product.get()); + } } @Override @@ -190,11 +188,11 @@ public final class WherePredicateStep<S> extends FilterStep<S> implements Scopin @Override public List<Traversal.Admin<S, ?>> getLocalChildren() { + if (this.predicateTraversals.isEmpty()) { + return (List) this.traversalRing.getTraversals(); + } final List<Traversal.Admin<S, ?>> children = new ArrayList<>((List) this.traversalRing.getTraversals()); - // Also expose predicate child traversals for strategy application and cloning - final List<Traversal.Admin<?, ?>> predicateTraversals = new ArrayList<>(); - P.collectTraversals(this.predicate, predicateTraversals); - children.addAll((List) predicateTraversals); + children.addAll((List) this.predicateTraversals); return children; } diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ChildTraversalVerificationStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ChildTraversalVerificationStrategy.java index 95ba0cc642..730de40913 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ChildTraversalVerificationStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/verification/ChildTraversalVerificationStrategy.java @@ -30,20 +30,12 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.filter.NoneStep; import org.apache.tinkerpop.gremlin.process.traversal.step.map.GraphStep; import org.apache.tinkerpop.gremlin.process.traversal.step.sideEffect.AddPropertyStepContract; import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy; -import org.apache.tinkerpop.gremlin.process.traversal.util.ChildTraversalContext; import org.apache.tinkerpop.gremlin.process.traversal.util.ChildTraversalValidator; /** - * A verification strategy that validates child traversals do not contain disallowed mutating steps - * based on the context in which they are used. This serves as a safety net for cases where - * construction-time validation is bypassed (e.g., programmatic traversal construction). - * <p> - * Context classification: - * <ul> - * <li>{@code HasStep}, {@code IsStep}, {@code AllStep}, {@code AnyStep}, {@code NoneStep} → FILTER</li> - * <li>{@code GraphStep} with idTraversal → LOOKUP</li> - * <li>{@code AddPropertyStepContract} → MUTATION</li> - * </ul> + * Validates that child traversals in filter, lookup, and mutation steps do not contain mutating steps. + * Serves as a safety net for programmatic traversal construction that bypasses the DSL's + * construction-time validation. */ public final class ChildTraversalVerificationStrategy extends AbstractTraversalStrategy<TraversalStrategy.VerificationStrategy> @@ -57,31 +49,23 @@ public final class ChildTraversalVerificationStrategy @Override public void apply(final Traversal.Admin<?, ?> traversal) { for (final Step<?, ?> step : traversal.getSteps()) { - if (step instanceof TraversalParent) { - final ChildTraversalContext context = classifyContext(step); - if (context != ChildTraversalContext.NONE) { - for (final Traversal.Admin<?, ?> child : ((TraversalParent) step).getLocalChildren()) { - try { - ChildTraversalValidator.validateRecursive(child, context); - } catch (final IllegalArgumentException e) { - throw new VerificationException(e.getMessage(), traversal); - } + if (step instanceof TraversalParent && hasChildTraversalsToValidate(step)) { + for (final Traversal.Admin<?, ?> child : ((TraversalParent) step).getLocalChildren()) { + try { + ChildTraversalValidator.validate(child); + } catch (final IllegalArgumentException e) { + throw new VerificationException(e.getMessage(), traversal); } } } } } - private ChildTraversalContext classifyContext(final Step<?, ?> step) { - if (step instanceof HasStep || step instanceof IsStep || - step instanceof AllStep || step instanceof AnyStep || step instanceof NoneStep) { - return ChildTraversalContext.FILTER; - } else if (step instanceof GraphStep && ((GraphStep<?, ?>) step).getIdTraversal() != null) { - return ChildTraversalContext.LOOKUP; - } else if (step instanceof AddPropertyStepContract) { - return ChildTraversalContext.MUTATION; - } - return ChildTraversalContext.NONE; + private boolean hasChildTraversalsToValidate(final Step<?, ?> step) { + return step instanceof HasStep || step instanceof IsStep || + step instanceof AllStep || step instanceof AnyStep || step instanceof NoneStep || + (step instanceof GraphStep && ((GraphStep<?, ?>) step).getIdTraversal() != null) || + step instanceof AddPropertyStepContract; } public static ChildTraversalVerificationStrategy instance() { diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidator.java index 18b35759a2..fb682e9a4b 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidator.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidator.java @@ -24,14 +24,10 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.Mutating; import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; /** - * Validates child traversals to ensure they do not contain disallowed steps based on the context - * in which they are used. This utility is shared between construction-time validation (in API methods) - * and strategy-time validation ({@code ChildTraversalVerificationStrategy}). - * <p> - * Validation rules: - * <ul> - * <li><b>FILTER / LOOKUP / MUTATION context:</b> No steps implementing {@link Mutating} are allowed at any nesting depth.</li> - * </ul> + * Validates that child traversals do not contain mutating steps. Child traversals used as + * arguments to filter predicates ({@code has()}, {@code is()}, etc.), lookup steps + * ({@code V(traversal)}, {@code E(traversal)}), and mutation steps ({@code property(traversal)}) + * must be read-only — their purpose is to compute values, not produce side effects. * <p> * Recursion walks both {@link TraversalParent#getLocalChildren()} and * {@link TraversalParent#getGlobalChildren()} to detect mutations nested inside @@ -40,64 +36,27 @@ import org.apache.tinkerpop.gremlin.process.traversal.step.TraversalParent; public final class ChildTraversalValidator { private ChildTraversalValidator() { - // static utility - } - - /** - * Validates a child traversal used in filter context (has, is, all, any, none, choose predicate). - * Throws {@link IllegalArgumentException} if any {@link Mutating} step is found. - */ - public static void validateFilterContext(final Traversal.Admin<?, ?> child) { - validateRecursive(child, ChildTraversalContext.FILTER); - } - - /** - * Validates a child traversal used in lookup context (V(traversal), E(traversal)). - * Throws {@link IllegalArgumentException} if any {@link Mutating} step is found. - */ - public static void validateLookupContext(final Traversal.Admin<?, ?> child) { - validateRecursive(child, ChildTraversalContext.LOOKUP); } /** - * Validates a child traversal used in mutation context (property(traversal)). - * Throws {@link IllegalArgumentException} if any {@link Mutating} step is found. + * Validates that a child traversal contains no {@link Mutating} steps at any nesting depth. + * Throws {@link IllegalArgumentException} if one is found. */ - public static void validateMutationContext(final Traversal.Admin<?, ?> child) { - validateRecursive(child, ChildTraversalContext.MUTATION); - } - - /** - * Recursively validates all steps in the child traversal and its nested children. - */ - public static void validateRecursive(final Traversal.Admin<?, ?> child, - final ChildTraversalContext context) { + public static void validate(final Traversal.Admin<?, ?> child) { for (final Step<?, ?> step : child.getSteps()) { - validateStep(step, context); + if (step instanceof Mutating) { + throw new IllegalArgumentException( + "Child traversal contains mutating step " + + step.getClass().getSimpleName() + ". Mutating steps are not allowed in child traversals."); + } if (step instanceof TraversalParent) { for (final Traversal.Admin<?, ?> nested : ((TraversalParent) step).getLocalChildren()) { - validateRecursive(nested, context); + validate(nested); } for (final Traversal.Admin<?, ?> nested : ((TraversalParent) step).getGlobalChildren()) { - validateRecursive(nested, context); + validate(nested); } } } } - - private static void validateStep(final Step<?, ?> step, final ChildTraversalContext context) { - switch (context) { - case FILTER: - case LOOKUP: - case MUTATION: - if (step instanceof Mutating) { - throw new IllegalArgumentException( - "Child traversal in " + context.name().toLowerCase() + " context contains mutating step " + - step.getClass().getSimpleName() + ". Mutating steps are not allowed in this context."); - } - break; - default: - break; - } - } }
