You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by ok...@apache.org on 2016/10/25 19:02:33 UTC

tinkerpop git commit: TraversalStrategy.GlobalCache.getStrategies() is now the point where static{} code blocks are loaded for Graph and GraphComputer classes. Added a test case to TraversalStrategiesTset that demonstrates that the GlobalCache can be man

Repository: tinkerpop
Updated Branches:
  refs/heads/TINKERPOP-1527 [created] f9bf0444e


TraversalStrategy.GlobalCache.getStrategies() is now the point where static{} code blocks are loaded for Graph and GraphComputer classes. Added a test case to TraversalStrategiesTset that demonstrates that the GlobalCache can be manipulated without fear of static{} re-registering strategies. This is a much much safer model and, moreover, proved correct via testing.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/f9bf0444
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/f9bf0444
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/f9bf0444

Branch: refs/heads/TINKERPOP-1527
Commit: f9bf0444e4bfe09dcea9b8dc05870ddaec4b6180
Parents: 285ff35
Author: Marko A. Rodriguez <ok...@gmail.com>
Authored: Tue Oct 25 13:02:29 2016 -0600
Committer: Marko A. Rodriguez <ok...@gmail.com>
Committed: Tue Oct 25 13:02:29 2016 -0600

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |   1 +
 .../decoration/VertexProgramStrategy.java       |   5 -
 .../process/traversal/TraversalStrategies.java  |  11 +-
 .../process/TraversalStrategiesTest.java        | 194 ++++++++++++++++++-
 4 files changed, 200 insertions(+), 11 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f9bf0444/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index a281c38..1c7441b 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 TinkerPop 3.2.4 (Release Date: NOT OFFICIALLY RELEASED YET)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+* Added a class loader to `TraversalStrategies.GlobalCache` which guarantees strategies are registered prior to `GlobalCache.getStrategies()`.
 * Fixed a severe bug where `GraphComputer` strategies are not being loaded until the second use of the traversal source.
 
 [[release-3-2-3]]

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f9bf0444/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/computer/traversal/strategy/decoration/VertexProgramStrategy.java
----------------------------------------------------------------------
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 0eeae3c..89e40cb 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
@@ -171,11 +171,6 @@ public final class VertexProgramStrategy extends AbstractTraversalStrategy<Trave
             }
         } else
             graphComputerClass = this.computer.getGraphComputerClass();
-        try {
-            Class.forName(graphComputerClass.getCanonicalName());
-        } catch (final ClassNotFoundException e) {
-            throw new IllegalStateException(e.getMessage(), e);
-        }
         final List<TraversalStrategy<?>> graphComputerStrategies = TraversalStrategies.GlobalCache.getStrategies(graphComputerClass).toList();
         traversalSource.getStrategies().addStrategies(graphComputerStrategies.toArray(new TraversalStrategy[graphComputerStrategies.size()]));
     }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f9bf0444/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalStrategies.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalStrategies.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalStrategies.java
index c271a37..015df70 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalStrategies.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalStrategies.java
@@ -225,7 +225,6 @@ public interface TraversalStrategies extends Serializable, Cloneable {
                     OrderLimitStrategy.instance(),
                     PathProcessorStrategy.instance(),
                     ComputerVerificationStrategy.instance());
-
             GRAPH_COMPUTER_CACHE.put(GraphComputer.class, graphComputerStrategies);
         }
 
@@ -239,6 +238,16 @@ public interface TraversalStrategies extends Serializable, Cloneable {
         }
 
         public static TraversalStrategies getStrategies(final Class graphOrGraphComputerClass) {
+            try {
+                // be sure to load the class so that its static{} traversal strategy registration component is loaded.
+                // this is more important for GraphComputer classes as they are typically not instantiated prior to strategy usage like Graph classes.
+                final String graphComputerClassName = null != graphOrGraphComputerClass.getDeclaringClass() ?
+                        graphOrGraphComputerClass.getCanonicalName().replace("." + graphOrGraphComputerClass.getSimpleName(), "$" + graphOrGraphComputerClass.getSimpleName()) :
+                        graphOrGraphComputerClass.getCanonicalName();
+                Class.forName(graphComputerClassName);
+            } catch (final ClassNotFoundException e) {
+                throw new IllegalStateException(e.getMessage(), e);
+            }
             if (Graph.class.isAssignableFrom(graphOrGraphComputerClass)) {
                 final TraversalStrategies traversalStrategies = GRAPH_CACHE.get(graphOrGraphComputerClass);
                 return null == traversalStrategies ? GRAPH_CACHE.get(Graph.class) : traversalStrategies;

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f9bf0444/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/TraversalStrategiesTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/TraversalStrategiesTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/TraversalStrategiesTest.java
index f316836..3d320db 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/TraversalStrategiesTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/TraversalStrategiesTest.java
@@ -18,26 +18,211 @@
  */
 package org.apache.tinkerpop.gremlin.process;
 
+import org.apache.commons.configuration.BaseConfiguration;
+import org.apache.commons.configuration.Configuration;
+import org.apache.tinkerpop.gremlin.process.computer.ComputerResult;
+import org.apache.tinkerpop.gremlin.process.computer.GraphComputer;
+import org.apache.tinkerpop.gremlin.process.computer.MapReduce;
+import org.apache.tinkerpop.gremlin.process.computer.VertexProgram;
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies;
 import org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategy;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.AbstractTraversalStrategy;
+import org.apache.tinkerpop.gremlin.structure.Edge;
+import org.apache.tinkerpop.gremlin.structure.Graph;
+import org.apache.tinkerpop.gremlin.structure.Transaction;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
 import org.junit.Test;
 
-import java.util.Arrays;
 import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collections;
+import java.util.Iterator;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.Future;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * @author Matthias Broecheler (me@matthiasb.com)
  */
 public class TraversalStrategiesTest {
 
+    @Test
+    public void shouldAllowUserManipulationOfGlobalCache() throws Exception {
+        ///////////
+        // GRAPH //
+        ///////////
+        TestGraph graph = new TestGraph();
+        TraversalStrategies strategies = graph.traversal().getStrategies();
+        assertFalse(TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList().isEmpty());
+        for (final TraversalStrategy strategy : TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()) {
+            assertTrue(strategies.getStrategy(strategy.getClass()).isPresent());
+        }
+        for (final TraversalStrategy strategy : TraversalStrategies.GlobalCache.getStrategies(TestGraphComputer.class).toList()) {
+            assertFalse(strategies.getStrategy(strategy.getClass()).isPresent());
+        }
+        assertTrue(strategies.getStrategy(StrategyA.class).isPresent());
+        assertTrue(strategies.getStrategy(StrategyB.class).isPresent());
+        assertFalse(strategies.getStrategy(StrategyC.class).isPresent());
+        assertFalse(strategies.getStrategy(StrategyD.class).isPresent());
+        strategies.addStrategies(new StrategyD());
+        strategies.removeStrategies(StrategyA.class);
+        assertFalse(strategies.getStrategy(StrategyA.class).isPresent());
+        assertTrue(strategies.getStrategy(StrategyD.class).isPresent());
+        ///
+        graph = new TestGraph();
+        strategies = graph.traversal().getStrategies();
+        for (final TraversalStrategy strategy : TraversalStrategies.GlobalCache.getStrategies(Graph.class).toList()) {
+            assertTrue(strategies.getStrategy(strategy.getClass()).isPresent());
+        }
+        for (final TraversalStrategy strategy : TraversalStrategies.GlobalCache.getStrategies(TestGraphComputer.class).toList()) {
+            assertFalse(strategies.getStrategy(strategy.getClass()).isPresent());
+        }
+        assertFalse(strategies.getStrategy(StrategyA.class).isPresent());
+        assertTrue(strategies.getStrategy(StrategyB.class).isPresent());
+        assertFalse(strategies.getStrategy(StrategyC.class).isPresent());
+        assertTrue(strategies.getStrategy(StrategyD.class).isPresent());
+        //////////////////////
+        /// GRAPH COMPUTER ///
+        //////////////////////
+        strategies = TraversalStrategies.GlobalCache.getStrategies(TestGraphComputer.class);
+        assertFalse(TraversalStrategies.GlobalCache.getStrategies(GraphComputer.class).toList().isEmpty());
+        for (final TraversalStrategy strategy : TraversalStrategies.GlobalCache.getStrategies(GraphComputer.class).toList()) {
+            assertTrue(strategies.getStrategy(strategy.getClass()).isPresent());
+        }
+        for (final TraversalStrategy strategy : TraversalStrategies.GlobalCache.getStrategies(TestGraph.class).toList()) {
+            assertFalse(strategies.getStrategy(strategy.getClass()).isPresent());
+        }
+        assertFalse(strategies.getStrategy(StrategyA.class).isPresent());
+        assertFalse(strategies.getStrategy(StrategyB.class).isPresent());
+        assertTrue(strategies.getStrategy(StrategyC.class).isPresent());
+        strategies.addStrategies(new StrategyE());
+        strategies.removeStrategies(StrategyC.class);
+        //
+        strategies = TraversalStrategies.GlobalCache.getStrategies(TestGraphComputer.class);
+        assertFalse(TraversalStrategies.GlobalCache.getStrategies(GraphComputer.class).toList().isEmpty());
+        for (final TraversalStrategy strategy : TraversalStrategies.GlobalCache.getStrategies(GraphComputer.class).toList()) {
+            assertTrue(strategies.getStrategy(strategy.getClass()).isPresent());
+        }
+        for (final TraversalStrategy strategy : TraversalStrategies.GlobalCache.getStrategies(TestGraph.class).toList()) {
+            assertFalse(strategies.getStrategy(strategy.getClass()).isPresent());
+        }
+        assertFalse(strategies.getStrategy(StrategyA.class).isPresent());
+        assertFalse(strategies.getStrategy(StrategyB.class).isPresent());
+        assertFalse(strategies.getStrategy(StrategyC.class).isPresent());
+        assertFalse(strategies.getStrategy(StrategyD.class).isPresent());
+        assertTrue(strategies.getStrategy(StrategyE.class).isPresent());
+    }
+
+    public static class TestGraphComputer implements GraphComputer {
+
+        static {
+            TraversalStrategies.GlobalCache.registerStrategies(TestGraphComputer.class,
+                    TraversalStrategies.GlobalCache.getStrategies(GraphComputer.class).clone().addStrategies(new StrategyC()));
+        }
+
+        @Override
+        public GraphComputer result(ResultGraph resultGraph) {
+            return this;
+        }
+
+        @Override
+        public GraphComputer persist(Persist persist) {
+            return this;
+        }
+
+        @Override
+        public GraphComputer program(VertexProgram vertexProgram) {
+            return this;
+        }
+
+        @Override
+        public GraphComputer mapReduce(MapReduce mapReduce) {
+            return this;
+        }
+
+        @Override
+        public GraphComputer workers(int workers) {
+            return this;
+        }
+
+        @Override
+        public GraphComputer vertices(Traversal<Vertex, Vertex> vertexFilter) throws IllegalArgumentException {
+            return this;
+        }
+
+        @Override
+        public GraphComputer edges(Traversal<Vertex, Edge> edgeFilter) throws IllegalArgumentException {
+            return this;
+        }
+
+        @Override
+        public Future<ComputerResult> submit() {
+            return new CompletableFuture<>();
+        }
+    }
+
+    public static class TestGraph implements Graph {
+
+        static {
+            TraversalStrategies.GlobalCache.registerStrategies(TestGraph.class,
+                    TraversalStrategies.GlobalCache.getStrategies(Graph.class).clone().addStrategies(new StrategyA(), new StrategyB()));
+        }
+
+        @Override
+        public Vertex addVertex(Object... keyValues) {
+            return null;
+        }
+
+        @Override
+        public <C extends GraphComputer> C compute(Class<C> graphComputerClass) throws IllegalArgumentException {
+            return (C) new TestGraphComputer();
+        }
+
+        @Override
+        public GraphComputer compute() throws IllegalArgumentException {
+            return new TestGraphComputer();
+        }
+
+        @Override
+        public Iterator<Vertex> vertices(Object... vertexIds) {
+            return Collections.emptyIterator();
+        }
+
+        @Override
+        public Iterator<Edge> edges(Object... edgeIds) {
+            return Collections.emptyIterator();
+        }
+
+        @Override
+        public Transaction tx() {
+            return null;
+        }
+
+        @Override
+        public void close() throws Exception {
+
+        }
+
+        @Override
+        public Variables variables() {
+            return null;
+        }
+
+        @Override
+        public Configuration configuration() {
+            return new BaseConfiguration();
+        }
+    }
+
     /**
      * Tests that {@link org.apache.tinkerpop.gremlin.process.traversal.TraversalStrategies#sortStrategies(java.util.List)}
      * works as advertised. This class defines a bunch of dummy strategies which define an order. It is verified
@@ -113,7 +298,7 @@ public class TraversalStrategiesTest {
         assertTrue(s.indexOf(a) < s.indexOf(b));
 
         // sort and then add more
-        s = new ArrayList<>((List)Arrays.asList(b,a,c));
+        s = new ArrayList<>((List) Arrays.asList(b, a, c));
         s = TraversalStrategies.sortStrategies(s);
         assertEquals(3, s.size());
         assertEquals(a, s.get(0));
@@ -220,7 +405,6 @@ public class TraversalStrategiesTest {
     }
 
 
-
     private static class DummyStrategy<S extends TraversalStrategy> extends AbstractTraversalStrategy<S> {
 
         @Override
@@ -262,7 +446,7 @@ public class TraversalStrategiesTest {
         assertEquals(e, s.get(3));
 
         //full reverse sorting
-        s = Arrays.asList(k,e,d,c,b,a);
+        s = Arrays.asList(k, e, d, c, b, a);
         s = TraversalStrategies.sortStrategies(s);
         assertEquals(6, s.size());
         assertEquals(a, s.get(0));