This is an automated email from the ASF dual-hosted git repository. spmallette pushed a commit to branch GValueManager3.8-wip in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit 9e66cd2922b36063833db1ade7910f4c36340e20 Author: Stephen Mallette <[email protected]> AuthorDate: Fri Jun 13 10:35:25 2025 -0400 wip --- .../strategy/decoration/VertexProgramStrategy.java | 2 +- .../gremlin/process/traversal/GValueManager.java | 2 +- .../traversal/strategy/GValueManagerVerifier.java | 66 ++++- .../strategy/GValueManagerVerifierTest.java | 278 +++++++++++++++++++++ 4 files changed, 343 insertions(+), 5 deletions(-) diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/strategy/decoration/VertexProgramStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/strategy/decoration/VertexProgramStrategy.java index 354eff9382..d28f6047c8 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/strategy/decoration/VertexProgramStrategy.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/strategy/decoration/VertexProgramStrategy.java @@ -80,7 +80,7 @@ public final class VertexProgramStrategy extends AbstractTraversalStrategy<Trave Step<?, ?> currentStep = traversal.getEndStep(); final Set<String> currentLabels = new HashSet<>(); while (!(currentStep instanceof EmptyStep)) { - if (currentStep instanceof VertexComputing && !(currentStep instanceof ProgramVertexProgramStep)) { // todo: is there a general solution? + if (currentStep instanceof VertexComputing && !(currentStep instanceof ProgramVertexProgramStep)) { currentLabels.addAll(currentStep.getLabels()); currentStep.clearLabels(); } else { diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GValueManager.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GValueManager.java index 3b08966c03..856af3b874 100644 --- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GValueManager.java +++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/GValueManager.java @@ -207,7 +207,7 @@ public final class GValueManager implements Serializable, Cloneable { private Map<Step, Set<GValue>> gatherStepGValues(final Traversal.Admin<?, ?> traversal, final Map<Step, Set<GValue>> result) { final GValueManager manager = traversal.getGValueManager(); - final Map<Step, Set<GValue>> r = null == result ? new HashMap<>() : result; + final Map<Step, Set<GValue>> r = null == result ? new IdentityHashMap<>() : result; for (Step step : traversal.getSteps()) { if (manager.isParameterized(step)) { diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifier.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifier.java index bbcb9b88c1..a6cdad3ad8 100644 --- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifier.java +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifier.java @@ -33,14 +33,17 @@ import org.apache.tinkerpop.gremlin.util.CollectionUtil; import java.util.Collection; import java.util.Collections; +import java.util.IdentityHashMap; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.stream.Collectors; import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.core.Is.is; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; /** * Provides utilities to verify the state and behavior of {@code GValueManager} during and after traversal strategy @@ -105,7 +108,7 @@ public class GValueManagerVerifier { public AfterVerifier<S, E> afterApplying() { // Capture pre-strategy state final GValueManager manager = traversal.getGValueManager(); - final Map<Step, Set<GValue>> preStepVariables = manager.gatherStepGValues(traversal); + final Map<Step, Set<GValue>> preStepGValues = manager.gatherStepGValues(traversal); final Set<String> preVariables = manager.variableNames(); final Set<GValue> preGValues = manager.gValues(); @@ -116,10 +119,10 @@ public class GValueManagerVerifier { } traversal.setStrategies(traversalStrategies); traversal.applyStrategies(); - + verifyStateIsConsistent(); - return new AfterVerifier<>(traversal, preVariables, preStepVariables, preGValues); + return new AfterVerifier<>(traversal, preVariables, preStepGValues, preGValues); } /** @@ -207,6 +210,63 @@ public class GValueManagerVerifier { assertEquals("All variables should be preserved", preVariables, currentVariables); return this; } + + /** + * Verifies that the contents of the GValueManager before and after applying strategies are unchanged. + * This ensures that they have the same Step references, StepContract objects, and the contents of the + * StepContracts are also the same. + */ + public AfterVerifier<S, E> notModified() { + // Verify that the same steps are in the manager + // Use identity-based sets for Step objects + final Set<Step> preSteps = Collections.newSetFromMap(new IdentityHashMap<>()); + preSteps.addAll(preStepGValues.keySet()); + + final Set<Step> currentSteps = Collections.newSetFromMap(new IdentityHashMap<>()); + currentSteps.addAll(manager.getSteps()); + + assertEquals("Steps in GValueManager should be unchanged", preSteps.size(), currentSteps.size()); + + for (Step preStep : preSteps) { + boolean found = false; + for (Step currentStep : currentSteps) { + if (preStep.toString().equals(currentStep.toString())) { + found = true; + + // Verify that the step has the same GValues + final Set<GValue> preGValuesForStep = preStepGValues.get(preStep); + final Set<GValue> currentGValuesForStep = manager.getGValues(currentStep).stream() + .collect(Collectors.toSet()); + assertEquals("GValues for step should be unchanged", preGValuesForStep, currentGValuesForStep); + + // Verify that the step has the same variables + final Set<String> preVariablesForStep = preStepVariables.get(preStep); + if (preVariablesForStep != null) { + final Set<String> currentVariablesForStep = manager.getGValues(currentStep).stream() + .filter(GValue::isVariable) + .map(GValue::getName) + .collect(Collectors.toSet()); + assertEquals("Variables for step should be unchanged", preVariablesForStep, currentVariablesForStep); + } + + // Verify that the step contract is the same if it exists + if (manager.isParameterized(currentStep)) { + final StepContract contract = manager.getStepContract(currentStep); + assertNotNull("Step contract should exist", contract); + } + + break; + } + } + assertTrue("Step not found in current steps: " + preStep, found); + } + + // Verify that the same GValues are in the manager + final Set<GValue> currentGValues = manager.gValues(); + assertEquals("GValues in GValueManager should be unchanged", preGValues, currentGValues); + + return this; + } } /** diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifierTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifierTest.java new file mode 100644 index 0000000000..2e4a92e409 --- /dev/null +++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/GValueManagerVerifierTest.java @@ -0,0 +1,278 @@ +/* + * 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.strategy; + +import org.apache.tinkerpop.gremlin.process.traversal.Traversal; +import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy; +import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.__; +import org.apache.tinkerpop.gremlin.process.traversal.step.GValue; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.HasStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.filter.RangeGlobalStep; +import org.apache.tinkerpop.gremlin.process.traversal.step.map.VertexStep; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.GValueManagerVerifier.AfterVerifier; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.GValueManagerVerifier.BeforeVerifier; +import org.apache.tinkerpop.gremlin.process.traversal.strategy.GValueManagerVerifier.VerificationBuilder; +import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper; + +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Set; + +import org.junit.Test; + +import static org.junit.Assert.assertNotNull; + +/** + * Tests for the GValueManagerVerifier class. + */ +public class GValueManagerVerifierTest { + + /** + * A simple strategy that doesn't modify the GValueManager state. + */ + private static final class NoOpStrategy extends AbstractTraversalStrategy<TraversalStrategy.VerificationStrategy> implements TraversalStrategy.VerificationStrategy { + private static final NoOpStrategy INSTANCE = new NoOpStrategy(); + + private NoOpStrategy() { + } + + @Override + public void apply(final Traversal.Admin<?, ?> traversal) { + // Do nothing + } + + public static NoOpStrategy instance() { + return INSTANCE; + } + } + + /** + * Tests the verify method with a single strategy. + */ + @Test + public void testVerifyWithSingleStrategy() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.filter(__.has("age", GValue.of("x", 25))).asAdmin(); + + // Verify that the verify method returns a VerificationBuilder + final VerificationBuilder<?, ?> builder = GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()); + assertNotNull("VerificationBuilder should not be null", builder); + } + + /** + * Tests the verify method with multiple strategies. + */ + @Test + public void testVerifyWithMultipleStrategies() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.filter(__.has("age", GValue.of("x", 25))).asAdmin(); + + // Create a collection of additional strategies + final Collection<TraversalStrategy> additionalStrategies = Collections.singletonList(NoOpStrategy.instance()); + + // Verify that the verify method returns a VerificationBuilder + final VerificationBuilder<?, ?> builder = GValueManagerVerifier.verify(traversal, NoOpStrategy.instance(), additionalStrategies); + assertNotNull("VerificationBuilder should not be null", builder); + } + + /** + * Tests the beforeApplying method of VerificationBuilder. + */ + @Test + public void testBeforeApplying() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.filter(__.has("age", GValue.of("x", 25))).asAdmin(); + + // Verify that the beforeApplying method returns a BeforeVerifier + final BeforeVerifier<?, ?> beforeVerifier = GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()).beforeApplying(); + assertNotNull("BeforeVerifier should not be null", beforeVerifier); + } + + /** + * Tests the afterApplying method of VerificationBuilder. + */ + @Test + public void testAfterApplying() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.filter(__.has("age", GValue.of("x", 25))).asAdmin(); + + // Verify that the afterApplying method returns an AfterVerifier + final AfterVerifier<?, ?> afterVerifier = GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()).afterApplying(); + assertNotNull("AfterVerifier should not be null", afterVerifier); + } + + /** + * Tests the afterApplying method of BeforeVerifier. + */ + @Test + public void testBeforeVerifierAfterApplying() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.filter(__.has("age", GValue.of("x", 25))).asAdmin(); + + // Verify that the afterApplying method of BeforeVerifier returns an AfterVerifier + final AfterVerifier<?, ?> afterVerifier = GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()) + .beforeApplying() + .afterApplying(); + assertNotNull("AfterVerifier should not be null", afterVerifier); + } + + /** + * Tests the variablesArePreserved method of AfterVerifier. + */ + @Test + public void testVariablesArePreserved() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.filter(__.has("age", GValue.of("x", 25))).asAdmin(); + + // Verify that variables are preserved after applying the NoOpStrategy + GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()) + .afterApplying() + .variablesArePreserved(); + } + + /** + * Tests the notModified method of AfterVerifier. + */ + @Test + public void testNotModified() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.filter(__.has("age", GValue.of("x", 25))).asAdmin(); + + // Verify that the state is unchanged after applying the NoOpStrategy + GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()) + .beforeApplying() + .stepsOfClassAreParameterized(true, HasStep.class) + .afterApplying() + .variablesArePreserved() + .notModified(); + } + + /** + * Tests the managerIsEmpty method of AbstractVerifier. + */ + @Test + public void testManagerIsEmpty() { + // Create a traversal without GValues + final Traversal.Admin<?, ?> traversal = __.out().asAdmin(); + + // Verify that the manager is empty + GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()) + .beforeApplying() + .managerIsEmpty(); + } + + /** + * Tests the stepsAreParameterized method of AbstractVerifier. + */ + @Test + public void testStepsAreParameterized() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.has("age", GValue.of("x", 25)).asAdmin(); + + // Get the HasStep from the traversal + final HasStep<?> hasStep = TraversalHelper.getStepsOfClass(HasStep.class, traversal).get(0); + + // Verify that the HasStep is parameterized + GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()) + .beforeApplying() + .stepsAreParameterized(true, hasStep); + } + + /** + * Tests the stepsOfClassAreParameterized method of AbstractVerifier. + */ + @Test + public void testStepsOfClassAreParameterized() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.filter(__.has("age", GValue.of("x", 25))).asAdmin(); + + // Verify that all HasSteps are parameterized + GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()) + .beforeApplying() + .stepsOfClassAreParameterized(true, HasStep.class); + } + + /** + * Tests the edgeLabelContractIsValid method of AbstractVerifier. + */ + @Test + public void testEdgeLabelContractIsValid() { + // Create a traversal with edge labels + final Traversal.Admin<?, ?> traversal = __.V().out(GValue.of("x", "knows"), GValue.of("y", "created")).asAdmin(); + + // Get the VertexStep from the traversal + final VertexStep<?> vertexStep = TraversalHelper.getStepsOfClass(VertexStep.class, traversal).get(0); + + // Verify that the EdgeLabelContract is valid + GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()) + .beforeApplying() + .edgeLabelContractIsValid(vertexStep, 2, + new String[]{"x", "y"}, + new String[]{"knows", "created"}); + } + + /** + * Tests the rangeContractIsValid method of AbstractVerifier. + */ + @Test + public void testRangeContractIsValid() { + // Create a traversal with a range step + final Traversal.Admin<?, ?> traversal = __.V().range(GValue.ofLong("low", 1L), GValue.ofLong("high", 10L)).asAdmin(); + + // Get the RangeGlobalStep from the traversal + final RangeGlobalStep<?> rangeStep = TraversalHelper.getStepsOfClass(RangeGlobalStep.class, traversal).get(0); + + // Verify that the RangeContract is valid + GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()) + .beforeApplying() + .rangeContractIsValid(rangeStep, 1, 10, "low", "high"); + } + + /** + * Tests the hasVariables method of AbstractVerifier with varargs. + */ + @Test + public void testHasVariablesVarargs() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.filter(__.has("age", GValue.of("x", 25))).asAdmin(); + + // Verify that the traversal has the expected variables + GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()) + .beforeApplying() + .hasVariables("x"); + } + + /** + * Tests the hasVariables method of AbstractVerifier with a Set. + */ + @Test + public void testHasVariablesSet() { + // Create a traversal with GValues + final Traversal.Admin<?, ?> traversal = __.filter(__.has("age", GValue.of("x", 25))).asAdmin(); + + // Create a set of expected variables + final Set<String> expectedVariables = new HashSet<>(Collections.singletonList("x")); + + // Verify that the traversal has the expected variables + GValueManagerVerifier.verify(traversal, NoOpStrategy.instance()) + .beforeApplying() + .hasVariables(expectedVariables); + } +}
