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 2020/11/05 19:05:53 UTC

[tinkerpop] branch TINKERPOP-2461 updated: TINKERPOP-2461 Aligned CoreImports with GroovyTranslator

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

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


The following commit(s) were added to refs/heads/TINKERPOP-2461 by this push:
     new c7ab06d  TINKERPOP-2461 Aligned CoreImports with GroovyTranslator
c7ab06d is described below

commit c7ab06d3892744fba190ab0ef22528485b293f3a
Author: Stephen Mallette <st...@amazon.com>
AuthorDate: Thu Nov 5 14:03:26 2020 -0500

    TINKERPOP-2461 Aligned CoreImports with GroovyTranslator
    
    This change only applied to the translator in gremlin-core. If folks want the old style they can use the deprecated one for now. The new style is more succinct and fits with the imports.
---
 CHANGELOG.asciidoc                                 |  2 +
 .../tinkerpop/gremlin/jsr223/CoreImports.java      | 13 ++++
 .../traversal/translator/GroovyTranslator.java     | 73 +++++++++++++---------
 .../traversal/translator/GroovyTranslatorTest.java | 28 ++++-----
 .../gremlin/groovy/loaders/StrategyLoader.groovy   |  5 ++
 5 files changed, 78 insertions(+), 43 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index f50c2e4..f41da2b 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,8 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Modified the text of `profile()` output to hide step instances injected for purpose of collecting metrics.
 * Bumped to Jackson 2.11.x.
 * Bumped Netty 4.1.52.
+* Provided a more concise syntax for constructing strategies in Groovy.
+* Aligned `CoreImports` with `GroovyTranslator` to generate more succinct syntax.
 * Moved `Translator` instances to `gremlin-core`.
 * Added `CheckedGraphManager` to prevent Gremlin Server from starting if there are no graphs configured.
 * Fixed bug in bytecode `Bindings` where calling `of()` prior to calling a child traversal in the same parent would cause the initial binding to be lost.
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/CoreImports.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/CoreImports.java
index cc7881c..1863c2a 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/CoreImports.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/CoreImports.java
@@ -136,6 +136,10 @@ import org.apache.tinkerpop.gremlin.structure.io.gryo.GryoWriter;
 import org.apache.tinkerpop.gremlin.structure.util.ElementHelper;
 import org.apache.tinkerpop.gremlin.structure.util.GraphFactory;
 import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
+import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceEdge;
+import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceProperty;
+import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertex;
+import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceVertexProperty;
 import org.apache.tinkerpop.gremlin.util.Gremlin;
 import org.apache.tinkerpop.gremlin.util.TimeUtil;
 import org.apache.tinkerpop.gremlin.util.function.Lambda;
@@ -166,6 +170,7 @@ public final class CoreImports {
         // CLASSES //
         /////////////
 
+        // structure
         CLASS_IMPORTS.add(Edge.class);
         CLASS_IMPORTS.add(Element.class);
         CLASS_IMPORTS.add(Graph.class);
@@ -175,6 +180,11 @@ public final class CoreImports {
         CLASS_IMPORTS.add(VertexProperty.class);
         CLASS_IMPORTS.add(GraphFactory.class);
         CLASS_IMPORTS.add(ElementHelper.class);
+        CLASS_IMPORTS.add(ReferenceEdge.class);
+        CLASS_IMPORTS.add(ReferenceProperty.class);
+        CLASS_IMPORTS.add(ReferenceVertex.class);
+        CLASS_IMPORTS.add(ReferenceVertexProperty.class);
+        
         // tokens
         CLASS_IMPORTS.add(SackFunctions.class);
         CLASS_IMPORTS.add(SackFunctions.Barrier.class);
@@ -298,6 +308,9 @@ public final class CoreImports {
         CLASS_IMPORTS.add(IteratorUtils.class);
         CLASS_IMPORTS.add(TimeUtil.class);
         CLASS_IMPORTS.add(Lambda.class);
+        CLASS_IMPORTS.add(java.util.Date.class);
+        CLASS_IMPORTS.add(java.sql.Timestamp.class);
+        CLASS_IMPORTS.add(java.util.UUID.class);
 
         /////////////
         // METHODS //
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java
index 1945e82..6e8dc48 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslator.java
@@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.process.traversal.translator;
 
 import org.apache.commons.configuration.ConfigurationConverter;
 import org.apache.commons.lang.StringEscapeUtils;
+import org.apache.tinkerpop.gremlin.jsr223.CoreImports;
 import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
 import org.apache.tinkerpop.gremlin.process.traversal.P;
 import org.apache.tinkerpop.gremlin.process.traversal.SackFunctions;
@@ -33,7 +34,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.strategy.TraversalStrategy
 import org.apache.tinkerpop.gremlin.process.traversal.util.ConnectiveP;
 import org.apache.tinkerpop.gremlin.process.traversal.util.OrP;
 import org.apache.tinkerpop.gremlin.structure.Edge;
-import org.apache.tinkerpop.gremlin.structure.Element;
 import org.apache.tinkerpop.gremlin.structure.Vertex;
 import org.apache.tinkerpop.gremlin.structure.VertexProperty;
 import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
@@ -51,6 +51,7 @@ import java.util.UUID;
 import java.util.function.BinaryOperator;
 import java.util.function.Supplier;
 import java.util.function.UnaryOperator;
+import java.util.stream.Collectors;
 
 /**
  * Converts bytecode to a Groovy string of Gremlin.
@@ -68,11 +69,11 @@ public final class GroovyTranslator implements Translator.ScriptTranslator {
         this.typeTranslator = typeTranslator;
     }
 
-    public static final GroovyTranslator of(final String traversalSource) {
+    public static GroovyTranslator of(final String traversalSource) {
         return of(traversalSource, null);
     }
 
-    public static final GroovyTranslator of(final String traversalSource, final TypeTranslator typeTranslator) {
+    public static GroovyTranslator of(final String traversalSource, final TypeTranslator typeTranslator) {
         return new GroovyTranslator(traversalSource,
                 Optional.ofNullable(typeTranslator).orElseGet(DefaultTypeTranslator::new));
     }
@@ -158,13 +159,13 @@ public final class GroovyTranslator implements Translator.ScriptTranslator {
             else if (object instanceof Integer)
                 return "(int) " + object;
             else if (object instanceof Class)
-                return ((Class) object).getCanonicalName();
+                return convertClassToString((Class<?>) object);
             else if (object instanceof Timestamp)
-                return "new java.sql.Timestamp(" + ((Timestamp) object).getTime() + ")";
+                return "new Timestamp(" + ((Timestamp) object).getTime() + ")";
             else if (object instanceof Date)
-                return "new java.util.Date(" + ((Date) object).getTime() + ")";
+                return "new Date(" + ((Date) object).getTime() + ")";
             else if (object instanceof UUID)
-                return "java.util.UUID.fromString('" + object.toString() + "')";
+                return "UUID.fromString('" + object.toString() + "')";
             else if (object instanceof P)
                 return convertPToString((P) object, new StringBuilder()).toString();
             else if (object instanceof SackFunctions.Barrier)
@@ -175,40 +176,36 @@ public final class GroovyTranslator implements Translator.ScriptTranslator {
                 return "TraversalOptionParent.Pick." + object.toString();
             else if (object instanceof Enum)
                 return ((Enum) object).getDeclaringClass().getSimpleName() + "." + object.toString();
-            else if (object instanceof Element) {
-                if (object instanceof Vertex) {
-                    final Vertex vertex = (Vertex) object;
-                    return "new org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex(" +
-                            convertToString(vertex.id()) + "," +
-                            convertToString(vertex.label()) + ", Collections.emptyMap())";
-                } else if (object instanceof Edge) {
+            else if (object instanceof Vertex) {
+                final Vertex vertex = (Vertex) object;
+                return "new ReferenceVertex(" +
+                        convertToString(vertex.id()) + "," +
+                        convertToString(vertex.label()) + ")";
+            } else if (object instanceof Edge) {
                     final Edge edge = (Edge) object;
-                    return "new org.apache.tinkerpop.gremlin.structure.util.detached.DetachedEdge(" +
+                    return "new ReferenceEdge(" +
                             convertToString(edge.id()) + "," +
                             convertToString(edge.label()) + "," +
-                            "Collections.emptyMap()," +
-                            convertToString(edge.outVertex().id()) + "," +
-                            convertToString(edge.outVertex().label()) + "," +
-                            convertToString(edge.inVertex().id()) + "," +
-                            convertToString(edge.inVertex().label()) + ")";
-                } else {// VertexProperty
-                    final VertexProperty vertexProperty = (VertexProperty) object;
-                    return "new org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertexProperty(" +
+                            "new ReferenceVertex(" + convertToString(edge.inVertex().id()) + "," +
+                            convertToString(edge.inVertex().label()) + ")," +
+                            "new ReferenceVertex(" + convertToString(edge.outVertex().id()) + "," +
+                            convertToString(edge.outVertex().label()) + "))";
+            } else if (object instanceof VertexProperty) {
+                    final VertexProperty<?> vertexProperty = (VertexProperty<?>) object;
+                    return "new ReferenceVertexProperty(" +
                             convertToString(vertexProperty.id()) + "," +
                             convertToString(vertexProperty.label()) + "," +
-                            convertToString(vertexProperty.value()) + "," +
-                            "Collections.emptyMap()," +
-                            convertToString(vertexProperty.element()) + ")";
-                }
+                            convertToString(vertexProperty.value()) + ")";
             } else if (object instanceof Lambda) {
                 final String lambdaString = ((Lambda) object).getLambdaScript().trim();
                 return lambdaString.startsWith("{") ? lambdaString : "{" + lambdaString + "}";
             } else if (object instanceof TraversalStrategyProxy) {
                 final TraversalStrategyProxy proxy = (TraversalStrategyProxy) object;
+                final String className = convertClassToString(proxy.getStrategyClass());
                 if (proxy.getConfiguration().isEmpty())
-                    return proxy.getStrategyClass().getCanonicalName() + ".instance()";
+                    return className;
                 else
-                    return proxy.getStrategyClass().getCanonicalName() + ".create(new org.apache.commons.configuration.MapConfiguration(" + convertToString(ConfigurationConverter.getMap(proxy.getConfiguration())) + "))";
+                    return String.format("new %s(%s)", className, convertMapToArguments(ConfigurationConverter.getMap(proxy.getConfiguration())));
             } else if (object instanceof TraversalStrategy) {
                 return convertToString(new TraversalStrategyProxy(((TraversalStrategy) object)));
             } else
@@ -275,6 +272,24 @@ public final class GroovyTranslator implements Translator.ScriptTranslator {
             current.append("TextP.").append(p.getBiPredicate().toString()).append("(").append(convertToString(p.getValue())).append(")");
             return current;
         }
+
+        /**
+         * Gets the string representation of a class with the default implementation simply checking to see if the
+         * {@code Class} is in {@link CoreImports} or not. If it is present that means it can be referenced using the
+         * simple name otherwise it uses the canonical name.
+         * <p/>
+         * Those building custom {@link ScriptTranslator} instances might override this if they have other classes
+         * that are not in {@link CoreImports} by default.
+         */
+        protected String convertClassToString(final Class<?> clazz) {
+            return CoreImports.getClassImports().contains(clazz) ? clazz.getSimpleName() : clazz.getCanonicalName();
+        }
+
+        private String convertMapToArguments(final Map<Object,Object> map) {
+            return map.entrySet().stream().map(entry ->
+                String.format("%s: %s", entry.getKey().toString(), convertToString(entry.getValue()))).
+                    collect(Collectors.joining(", "));
+        }
     }
 }
 
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java
index b370d73..2575b3a 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/translator/GroovyTranslatorTest.java
@@ -63,7 +63,7 @@ public class GroovyTranslatorTest {
 
     @Test
     public void shouldTranslateStrategies() throws Exception {
-        assertEquals("g.withStrategies(org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.ReadOnlyStrategy.instance(),org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy.create(new org.apache.commons.configuration.MapConfiguration([(\"checkAdjacentVertices\"):(false),(\"vertices\"):(__.hasLabel(\"person\"))]))).V().has(\"name\")",
+        assertEquals("g.withStrategies(ReadOnlyStrategy,new SubgraphStrategy(checkAdjacentVertices: false, vertices: __.hasLabel(\"person\"))).V().has(\"name\")",
                 translator.translate(g.withStrategies(ReadOnlyStrategy.instance(),
                         SubgraphStrategy.build().checkAdjacentVertices(false).vertices(hasLabel("person")).create()).
                         V().has("name").asAdmin().getBytecode()));
@@ -133,7 +133,7 @@ public class GroovyTranslatorTest {
         final Calendar c = Calendar.getInstance();
         c.set(1975, Calendar.SEPTEMBER, 7);
         final Date d = c.getTime();
-        assertTranslation(String.format("new java.util.Date(%s)", d.getTime()), d);
+        assertTranslation(String.format("new Date(%s)", d.getTime()), d);
     }
 
     @Test
@@ -141,13 +141,13 @@ public class GroovyTranslatorTest {
         final Calendar c = Calendar.getInstance();
         c.set(1975, Calendar.SEPTEMBER, 7);
         final Timestamp t = new Timestamp(c.getTime().getTime());
-        assertTranslation(String.format("new java.sql.Timestamp(%s)", t.getTime()), t);
+        assertTranslation(String.format("new Timestamp(%s)", t.getTime()), t);
     }
 
     @Test
     public void shouldTranslateUuid() {
         final UUID uuid = UUID.fromString("ffffffff-fd49-1e4b-0000-00000d4b8a1d");
-        assertTranslation(String.format("java.util.UUID.fromString('%s')", uuid), uuid);
+        assertTranslation(String.format("UUID.fromString('%s')", uuid), uuid);
     }
 
     @Test
@@ -218,18 +218,18 @@ public class GroovyTranslatorTest {
         final Vertex vertex1 = DetachedVertex.build().setLabel("customer").setId(id1)
                 .create();
         final String script1 = translator.translate(g.inject(vertex1).asAdmin().getBytecode());
-        assertEquals("g.inject(new org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex(" +
+        assertEquals("g.inject(new ReferenceVertex(" +
                         "\"customer:10:foo bar \\$100#90\"," +
-                        "\"customer\", Collections.emptyMap()))",
+                        "\"customer\"))",
                 script1);
 
         final Object id2 = "user:20:foo\\u0020bar\\u005c\\u0022mr\\u005c\\u0022\\u00241000#50"; // user:20:foo\u0020bar\u005c\u0022mr\u005c\u0022\u00241000#50
         final Vertex vertex2 = DetachedVertex.build().setLabel("user").setId(id2)
                 .create();
         final String script2 = translator.translate(g.inject(vertex2).asAdmin().getBytecode());
-        assertEquals("g.inject(new org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex(" +
+        assertEquals("g.inject(new ReferenceVertex(" +
                         "\"user:20:foo\\\\u0020bar\\\\u005c\\\\u0022mr\\\\u005c\\\\u0022\\\\u00241000#50\"," +
-                        "\"user\", Collections.emptyMap()))",
+                        "\"user\"))",
                 script2);
 
         final Object id3 = "knows:30:foo\u0020bar\u0020\u0024100:\\u0020\\u0024500#70";
@@ -238,19 +238,19 @@ public class GroovyTranslatorTest {
                 .setInV((DetachedVertex) vertex2)
                 .create();
         final String script3 = translator.translate(g.inject(edge).asAdmin().getBytecode());
-        assertEquals("g.inject(new org.apache.tinkerpop.gremlin.structure.util.detached.DetachedEdge(" +
+        assertEquals("g.inject(new ReferenceEdge(" +
                         "\"knows:30:foo bar \\$100:\\\\u0020\\\\u0024500#70\"," +
-                        "\"knows\",Collections.emptyMap()," +
-                        "\"customer:10:foo bar \\$100#90\",\"customer\"," +
-                        "\"user:20:foo\\\\u0020bar\\\\u005c\\\\u0022mr\\\\u005c\\\\u0022\\\\u00241000#50\",\"user\"))",
+                        "\"knows\"," +
+                        "new ReferenceVertex(\"user:20:foo\\\\u0020bar\\\\u005c\\\\u0022mr\\\\u005c\\\\u0022\\\\u00241000#50\",\"user\")," +
+                        "new ReferenceVertex(\"customer:10:foo bar \\$100#90\",\"customer\")))",
                 script3);
 
         final String script4 = translator.translate(
                 g.addE("knows").from(vertex1).to(vertex2).property("when", "2018/09/21")
                         .asAdmin().getBytecode());
         assertEquals("g.addE(\"knows\")" +
-                        ".from(new org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex(\"customer:10:foo bar \\$100#90\",\"customer\", Collections.emptyMap()))" +
-                        ".to(new org.apache.tinkerpop.gremlin.structure.util.detached.DetachedVertex(\"user:20:foo\\\\u0020bar\\\\u005c\\\\u0022mr\\\\u005c\\\\u0022\\\\u00241000#50\",\"user\", Collections.emptyMap()))" +
+                        ".from(new ReferenceVertex(\"customer:10:foo bar \\$100#90\",\"customer\"))" +
+                        ".to(new ReferenceVertex(\"user:20:foo\\\\u0020bar\\\\u005c\\\\u0022mr\\\\u005c\\\\u0022\\\\u00241000#50\",\"user\"))" +
                         ".property(\"when\",\"2018/09/21\")",
                 script4);
     }
diff --git a/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/loaders/StrategyLoader.groovy b/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/loaders/StrategyLoader.groovy
index 5396737..b9651fd 100644
--- a/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/loaders/StrategyLoader.groovy
+++ b/gremlin-groovy/src/main/groovy/org/apache/tinkerpop/gremlin/groovy/loaders/StrategyLoader.groovy
@@ -19,6 +19,7 @@
 package org.apache.tinkerpop.gremlin.groovy.loaders
 
 import org.apache.commons.configuration.MapConfiguration
+import org.apache.tinkerpop.gremlin.process.computer.traversal.strategy.decoration.VertexProgramStrategy
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.ElementIdStrategy
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.HaltedTraverserStrategy
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy
@@ -52,11 +53,13 @@ class StrategyLoader {
         // SackStrategy.metaClass.constructor << { Map conf -> SackStrategy.create(new MapConfiguration(conf)) }
         // # SideEffectStrategy is internal
         SubgraphStrategy.metaClass.constructor << { Map conf -> SubgraphStrategy.create(new MapConfiguration(conf)) }
+        VertexProgramStrategy.metaClass.constructor << { Map conf -> VertexProgramStrategy.create(new MapConfiguration(conf)) }
 
         // finalization
         MatchAlgorithmStrategy.metaClass.constructor << { Map conf -> MatchAlgorithmStrategy.create(new MapConfiguration(conf)) }
         // # ProfileStrategy is singleton/internal
         // # ReferenceElementStrategy is singleton/internal
+        // # ComputerFinalizationStrategy is singleton/internal
 
         // optimization
         // # AdjacentToIncidentStrategy is singleton/internal
@@ -72,6 +75,8 @@ class StrategyLoader {
         // # PathProcessorStrategy is singleton/internal
         // # PathRetractionStrategy is singleton/internal
         // # RepeatUnrollStrategy is singleton/internal
+        // # GraphFilterStrategy is singleton/internal
+        // # MessagePassingReductionStrategy is singleton/internal
 
         // verification
         // # ComputerVerificationStrategy is singleton/internal