You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2022/06/28 14:12:17 UTC

[tinkerpop] branch fix-empty-graph created (now 8a7049f568)

This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a change to branch fix-empty-graph
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


      at 8a7049f568 wip

This branch includes the following new commits:

     new 8a7049f568 wip

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[tinkerpop] 01/01: wip

Posted by sp...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

spmallette pushed a commit to branch fix-empty-graph
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit 8a7049f5685c57bf6b523f41b8b3025523e89b52
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Tue Jun 28 10:11:58 2022 -0400

    wip
---
 .../strategy/decoration/PartitionStrategy.java     |  2 +-
 .../process/traversal/util/DefaultTraversal.java   | 11 ++++-
 .../tinkergraph/structure/TinkerGraphTest.java     | 56 ++++++++++++++++++++++
 3 files changed, 67 insertions(+), 2 deletions(-)

diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java
index e0920cf3ba..d3d6147e10 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/strategy/decoration/PartitionStrategy.java
@@ -126,7 +126,7 @@ public final class PartitionStrategy extends AbstractTraversalStrategy<Traversal
         // writes.
         final Optional<Graph.Features.VertexFeatures> vertexFeatures;
         if (includeMetaProperties) {
-            final Graph graph = TraversalHelper.getRootTraversal(traversal).getGraph().orElseThrow(
+            final Graph graph = traversal.getGraph().orElseThrow(
                     () -> new IllegalStateException("PartitionStrategy does not work with anonymous Traversals when includeMetaProperties is enabled"));
             final Graph.Features.VertexFeatures vf = graph.features().vertex();
             final boolean supportsMetaProperties = vf.supportsMetaProperties();
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
index a6b77ea7d2..4f314dcf8b 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversal.java
@@ -149,7 +149,16 @@ public class DefaultTraversal<S, E> implements Traversal.Admin<S, E> {
             final Iterator<TraversalStrategy<?>> strategyIterator = this.strategies.iterator();
             while (strategyIterator.hasNext()) {
                 final TraversalStrategy<?> strategy = strategyIterator.next();
-                TraversalHelper.applyTraversalRecursively(strategy::apply, this);
+                TraversalHelper.applyTraversalRecursively(t -> {
+                    strategy.apply(t);
+
+                    // after the strategy is applied, it may have modified the traversal where a new traversal object
+                    // was added. if the strategy didn't set the Graph object it could leave that new traversal in a
+                    // state where another strategy might fail if that dependency is not satisfied
+                    TraversalHelper.applyTraversalRecursively(i -> {
+                        if (hasGraph) i.setGraph(this.graph);
+                    }, this);
+                }, this);
             }
 
             // don't need to re-apply strategies to "this" - leads to endless recursion in GraphComputer.
diff --git a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java
index a6611bd398..5c8654f9fc 100644
--- a/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java
+++ b/tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphTest.java
@@ -28,8 +28,13 @@ 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.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.strategy.AbstractTraversalStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.CountStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.optimization.IdentityRemovalStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ComputerVerificationStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReservedKeysVerificationStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.VerificationException;
 import org.apache.tinkerpop.gremlin.process.traversal.util.Metrics;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalHelper;
 import org.apache.tinkerpop.gremlin.process.traversal.util.TraversalMetrics;
@@ -51,6 +56,7 @@ import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoClassResolverV1d0;
 import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoMapper;
 import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoVersion;
 import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoWriter;
+import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
 import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
 import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper;
 import org.apache.tinkerpop.shaded.kryo.ClassResolver;
@@ -70,6 +76,7 @@ import java.io.FileOutputStream;
 import java.io.InputStream;
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
@@ -879,6 +886,28 @@ public class TinkerGraphTest {
         assertEquals(3, traversal().withEmbedded(graph).V().hasLabel("person").count().next().intValue());
     }
 
+    /**
+     * Basically just trying to validate through {@link CountStrategy} that a child traversal constructed there gets
+     * its {@link Graph} instance set. By using {@link AssertGraphStrategy} an exception can get triggered in betweeen
+     * strategy applications to validate that the {@code Graph} is assigned.
+     */
+    @Test
+    public void shouldSetGraphBetweenStrategyApplicationsForNewChildTraversalsConstructedByStrategies() throws Exception {
+        final GraphTraversalSource g = TinkerGraph.open().traversal();
+        g.addV("node").property(T.id, 1).as("1")
+                .addV("node").property(T.id, 2).as("2")
+                .addV("node").property(T.id, 3).as("3")
+                .addV("node").property(T.id, 4).as("4")
+                .addE("child").from("1").to("2")
+                .addE("child").from("2").to("3")
+                .addE("child").from("4").to("3").iterate();
+
+        // this just needs to not throw an exception for it to pass
+        g.withStrategies(AssertGraphStrategy.instance()).V(3).repeat(__.inE("child").outV().simplePath())
+                .until(__.or(__.inE().count().is(0), __.loops().is(P.eq(2))))
+                .path().count().next();
+    }
+
     /**
      * Coerces a {@code Color} to a {@link TinkerGraph} during serialization.  Demonstrates how custom serializers
      * can be developed that can coerce one value to another during serialization.
@@ -990,4 +1019,31 @@ public class TinkerGraphTest {
             return false;
         }
     }
+
+    /**
+     * Validates that a {@link Graph} is assigned to each {@link Traversal} if it is expected.
+     */
+    public static class AssertGraphStrategy extends AbstractTraversalStrategy<TraversalStrategy.VerificationStrategy> implements TraversalStrategy.VerificationStrategy {
+
+        public static final AssertGraphStrategy INSTANCE = new AssertGraphStrategy();
+
+        private AssertGraphStrategy() {}
+
+        @Override
+        public void apply(final Traversal.Admin<?, ?> traversal) {
+            if (!(traversal instanceof AbstractLambdaTraversal) && (!traversal.getGraph().isPresent()
+                    || traversal.getGraph().get().equals(EmptyGraph.instance())))
+                throw new VerificationException("Graph object not set on Traversal", traversal);
+        }
+
+        @Override
+        public Set<Class<? extends VerificationStrategy>> applyPost() {
+            return Collections.singleton(ComputerVerificationStrategy.class);
+        }
+
+        public static AssertGraphStrategy instance() {
+            return INSTANCE;
+        }
+
+    }
 }