This is an automated email from the ASF dual-hosted git repository. xiazcy pushed a commit to branch steps-taking-traversal in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 0bac231927d01630c86373c7186ae1764bce3ef0 Author: Yang Xia <[email protected]> AuthorDate: Fri Jun 12 10:41:32 2026 -0700 Eliminate leftover null guards and dead code from HasContainer unification Assisted-by: Kiro:claude-opus-4-6 --- .../process/traversal/step/HasContainerHolder.java | 2 +- .../process/traversal/step/filter/HasStep.java | 4 +- .../traversal/step/sideEffect/AddPropertyStep.java | 11 +----- .../process/traversal/step/util/HasContainer.java | 6 +-- .../GremlinLangTraversalRoundTripTest.java | 7 +--- .../gremlin/process/traversal/PTraversalTest.java | 44 ++++++++++------------ .../step/CloneIndependenceTraversalTest.java | 2 - .../traversal/step/util/HasContainerTest.java | 1 - .../util/ChildTraversalValidatorTest.java | 2 +- 9 files changed, 29 insertions(+), 50 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/HasContainerHolder.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/HasContainerHolder.java index 47343d3ea6..f016aa03f4 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/HasContainerHolder.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/HasContainerHolder.java @@ -53,7 +53,7 @@ public interface HasContainerHolder<S, E> extends GValueHolder<S, E> { } public default Collection<P<?>> getPredicatesGValueSafe() { - return getHasContainers().stream().map(p -> p.getPredicate()).filter(p -> p != null).collect(Collectors.toList()); + return getHasContainers().stream().map(p -> p.getPredicate()).collect(Collectors.toList()); } public default HasContainerHolder<S, E> asConcreteStep() { diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStep.java index 1b56dbdfeb..a69e8f7871 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStep.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/filter/HasStep.java @@ -131,7 +131,7 @@ public class HasStep<S extends Element> extends FilterStep<S> implements HasCont * {@link #setTraversal(Traversal.Admin)} which is the single integration point. */ private void collectChildTraversals(final HasContainer hc) { - if (hc.getPredicate() != null && hc.getPredicate().hasTraversal()) { + if (hc.getPredicate().hasTraversal()) { P.collectTraversals(hc.getPredicate(), this.childTraversals); } } @@ -182,7 +182,7 @@ public class HasStep<S extends Element> extends FilterStep<S> implements HasCont for (final HasContainer hasContainer : this.hasContainers) { final HasContainer clonedHc = hasContainer.clone(); clone.hasContainers.add(clonedHc); - if (clonedHc.getPredicate() != null && clonedHc.getPredicate().hasTraversal()) { + if (clonedHc.getPredicate().hasTraversal()) { P.collectTraversals(clonedHc.getPredicate(), clone.childTraversals); } } 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 7b5acbf44a..0901a67196 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 @@ -49,21 +49,12 @@ import java.util.Set; */ public class AddPropertyStep<S extends Element> extends SideEffectStep<S> implements AddPropertyStepContract<S> { - /** - * Internal placeholder key used for the single-argument {@code property(traversal)} (Map-producing) form, - * where the property keys are supplied by the traversal's resulting Map rather than by a caller-provided key. - * This value is never used for dispatch (see {@link #mapForm}); it exists only to give the parameter a - * stable, non-null key within {@link org.apache.tinkerpop.gremlin.process.traversal.step.util.Parameters}. - */ - static final String MAP_VALUE_TRAVERSAL_KEY = "~traversalMap"; - private Parameters internalParameters = new Parameters(); private Parameters withConfiguration = new Parameters(); private final VertexProperty.Cardinality cardinality; /** * Marks the single-argument {@code property(traversal)} form whose child traversal must produce a Map of - * property key/value pairs. Dispatch is driven by this flag rather than by inspecting the property key, so a - * caller-supplied key that happens to collide with {@link #MAP_VALUE_TRAVERSAL_KEY} is handled normally. + * property key/value pairs. Dispatch is driven by this flag rather than by inspecting the property key. */ private final boolean mapForm; private CallbackRegistry<Event.ElementPropertyChangedEvent> callbackRegistry; diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java index c6d0d1fe29..82204d49f2 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainer.java @@ -110,7 +110,7 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element> public HasContainer clone() { try { final HasContainer clone = (HasContainer) super.clone(); - clone.predicate = this.predicate != null ? this.predicate.clone() : null; + clone.predicate = this.predicate.clone(); return clone; } catch (final CloneNotSupportedException e) { throw new IllegalStateException(e.getMessage(), e); @@ -119,7 +119,7 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element> @Override public int hashCode() { - return (this.key != null ? this.key.hashCode() : 0) ^ (this.predicate != null ? this.predicate.hashCode() : 0); + return (this.key != null ? this.key.hashCode() : 0) ^ this.predicate.hashCode(); } public final String getKey() { @@ -147,7 +147,7 @@ public class HasContainer implements Serializable, Cloneable, Predicate<Element> * at runtime (e.g. {@code P.eq(traversal)} or {@code P.within(traversal)}). */ public boolean hasTraversal() { - return this.predicate != null && this.predicate.hasTraversal(); + return this.predicate.hasTraversal(); } //////////// diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTraversalRoundTripTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTraversalRoundTripTest.java index 391938b9ca..ff1cc9a6a7 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTraversalRoundTripTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/GremlinLangTraversalRoundTripTest.java @@ -29,12 +29,9 @@ import static org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalS import static org.junit.Assert.assertEquals; /** - * Property 7: GremlinLang serialization round-trip preserves traversal arguments. - * <p> + * Tests that GremlinLang serialization round-trips preserve traversal arguments. * For any step containing a child traversal argument, serializing to GremlinLang and parsing back - * SHALL produce a structurally equivalent traversal. - * <p> - * <b>Validates: Requirements 1.3, 3.6, 4.5, 6.5, 7.3</b> + * produces a structurally equivalent traversal. */ public class GremlinLangTraversalRoundTripTest { diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTraversalTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTraversalTest.java index d52b45ae49..86c82c2b22 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTraversalTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/PTraversalTest.java @@ -26,6 +26,9 @@ import org.junit.experimental.runners.Enclosed; import org.junit.runner.RunWith; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.CoreMatchers.notNullValue; +import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.CoreMatchers.sameInstance; import static org.hamcrest.MatcherAssert.assertThat; /** @@ -36,13 +39,8 @@ import static org.hamcrest.MatcherAssert.assertThat; public class PTraversalTest { /** - * Property 10: Traversal detection in predicates is accurate. - * <p> - * For any P instance containing a child traversal, {@code P.hasTraversal()} SHALL return true. - * For any P instance containing only literal values or GValue variables, - * {@code P.hasTraversal()} SHALL return false. - * <p> - * <b>Validates: Requirements 9.4</b> + * Tests that traversal detection in predicates is accurate: {@code P.hasTraversal()} returns + * true for traversal-bearing predicates and false for literal/GValue predicates. */ public static class TraversalDetectionTest { @@ -74,25 +72,20 @@ public class PTraversalTest { public void shouldReturnTraversalValueWhenPresent() { final Traversal.Admin<?, ?> traversal = __.inject(42).asAdmin(); final P<Object> p = P.eq(traversal); - assertThat(p.getTraversalValue() == traversal, is(true)); + assertThat(p.getTraversalValue(), is(sameInstance(traversal))); } @Test public void shouldReturnNullTraversalValueForLiteral() { final P<String> p = P.eq("value"); - assertThat(p.getTraversalValue() == null, is(true)); + assertThat(p.getTraversalValue(), is(nullValue())); } } /** - * Property 3: Single-value predicate rejects multiple traversal results. - * <p> - * For any single-value predicate (eq, neq, gt, lt, gte, lte) and any child traversal that produces - * more than one result, the predicate SHALL throw an IllegalArgumentException. - * For any collection predicate (within, without) and any child traversal producing multiple results, - * the predicate SHALL accept all results as the collection value. - * <p> - * <b>Validates: Requirements 2.5</b> + * Tests first-result semantics: single-value predicates (eq, neq, gt, lt, gte, lte) take the + * first result from a multi-result traversal. Collection predicates (within, without) accept + * all results as the collection value. */ public static class SingleValuePredicateRejectionTest { @@ -100,8 +93,6 @@ public class PTraversalTest { return new B_O_Traverser<>(value, 1L); } - // --- Single-value predicates should throw on multiple results --- - // --- Single-value predicates take first result, ignore extras (consistent with by()) --- @SuppressWarnings("unchecked") @@ -267,7 +258,7 @@ public class PTraversalTest { @Test public void shouldReturnTraversalValuesListForMultiTraversal() { final P<Object> p = P.within(__.constant(1).asAdmin(), __.constant(2).asAdmin()); - assertThat(p.getTraversalValues() != null, is(true)); + assertThat(p.getTraversalValues(), is(notNullValue())); assertThat(p.getTraversalValues().size(), is(2)); } @@ -275,7 +266,7 @@ public class PTraversalTest { public void shouldReturnNullTraversalValueForMultiTraversal() { // Single traversalValue should be null when using multi-traversal form final P<Object> p = P.within(__.constant(1).asAdmin(), __.constant(2).asAdmin()); - assertThat(p.getTraversalValue() == null, is(true)); + assertThat(p.getTraversalValue(), is(nullValue())); } // --- Resolution and testing --- @@ -361,14 +352,17 @@ public class PTraversalTest { // Clone should have independent traversal values assertThat(clone.hasTraversal(), is(true)); - assertThat(clone.getTraversalValues() != null, is(true)); + assertThat(clone.getTraversalValues(), is(notNullValue())); assertThat(clone.getTraversalValues().size(), is(2)); - // Modifying clone should not affect original + // Resolve the clone - this mutates the clone's internal state clone.resolve(createTraverser("start")); assertThat(clone.test(1), is(true)); - // Original should still be unresolved (literals empty) - assertThat(original.getTraversalValues().size(), is(2)); + + // Original should be unaffected: resolving it independently should still work correctly + original.resolve(createTraverser("other")); + assertThat(original.test(1), is(true)); + assertThat(original.test(2), is(true)); } // --- Varargs detection --- diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/CloneIndependenceTraversalTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/CloneIndependenceTraversalTest.java index 09af6da179..4c8d6ff850 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/CloneIndependenceTraversalTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/CloneIndependenceTraversalTest.java @@ -42,8 +42,6 @@ import static org.hamcrest.core.IsSame.sameInstance; * For any step containing child traversal arguments, cloning the step SHALL produce a deep copy * where modifying the clone's child traversal state does NOT affect the original step's child * traversal state. - * <p> - * <b>Validates: Requirements 6.2</b> */ public class CloneIndependenceTraversalTest { diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainerTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainerTest.java index 4049b20eb6..efe7072ad0 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainerTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/step/util/HasContainerTest.java @@ -56,7 +56,6 @@ public class HasContainerTest { final HasContainer hc = new HasContainer("age", P.eq(traversal)); assertEquals("age", hc.getKey()); - assertThat(hc.getPredicate(), is(notNullValue())); assertThat(hc.getPredicate().getTraversalValue(), is(sameInstance(traversal))); } diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidatorTest.java index 68b99296ec..3960539d78 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidatorTest.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/util/ChildTraversalValidatorTest.java @@ -216,7 +216,7 @@ public class ChildTraversalValidatorTest { @Test public void shouldRejectAddVInMutationContext() { - // addV is now blocked in property(traversal) - all mutating steps are blocked in all contexts + // addV is blocked in property(traversal) - mutating steps are blocked in child traversals assertThrows(IllegalArgumentException.class, () -> g.V().property(__.V().addV("temp").project("k").by("name"))); }
