wip - params

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinLangScriptEngine.java

@@ -33,7 +33,11 @@
 import javax.script.SimpleBindings;
 import java.io.IOException;
 import java.io.Reader;
+import java.util.Arrays;
+import java.util.List;
 import java.util.Map;
+import java.util.Optional;
+import java.util.function.Function;
 
 /**
  * A {@link GremlinScriptEngine} implementation that evaluates Gremlin scripts using {@code gremlin-language}. As it
@@ -53,6 +57,8 @@
 public class GremlinLangScriptEngine extends AbstractScriptEngine implements GremlinScriptEngine {
     private volatile GremlinScriptEngineFactory factory;
 
+    private final Function<Map<String, Object>, VariableResolver> variableResolverMaker;
+
     /**
      * Creates a new instance using no {@link Customizer}.
      */
@@ -61,6 +67,15 @@ public GremlinLangScriptEngine() {
     }
 
     public GremlinLangScriptEngine(final Customizer... customizers) {
+        final List<Customizer> listOfCustomizers = Arrays.asList(customizers);
+
+        // this ScriptEngine really only supports the VariableResolverCustomizer to configure the VariableResolver
+        // and can't configure it more than once. first one wins
+        final Optional<Customizer> opt = listOfCustomizers.stream().filter(c -> c instanceof VariableResolverCustomizer).findFirst();
+        variableResolverMaker = opt.isPresent() ?
+                ((VariableResolverCustomizer) opt.get()).getVariableResolverMaker() :
+                VariableResolver.DirectVariableResolver::new;
+
     }
 
     @Override
@@ -108,7 +123,7 @@ public Object eval(final String script, final ScriptContext context) throws Scri
 
         final Map<String, Object> m = context.getBindings(ScriptContext.ENGINE_SCOPE);
         final GremlinAntlrToJava antlr = new GremlinAntlrToJava((GraphTraversalSource) o,
-                new VariableResolver.DefaultVariableResolver(m));
+                variableResolverMaker.apply(m));
 
         try {
             return GremlinQueryParser.parse(script, antlr);

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/VariableResolverCustomizer.java

@@ -0,0 +1,40 @@
+/*
+ * 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.jsr223;
+
+import org.apache.tinkerpop.gremlin.language.grammar.VariableResolver;
+
+import java.util.Map;
+import java.util.function.Function;
+
+/**
+ * Supplies a {@link VariableResolver} implementation to the {@link GremlinLangScriptEngine}. This {@link Customizer}
+ * is not relevant to any other {@link GremlinScriptEngine} implementation.
+ */
+public class VariableResolverCustomizer implements Customizer {
+    private final Function<Map<String,Object>, VariableResolver> variableResolverMaker;
+
+    public VariableResolverCustomizer(final Function<Map<String, Object>, VariableResolver> variableResolverMaker) {
+        this.variableResolverMaker = variableResolverMaker;
+    }
+
+    public Function<Map<String, Object>, VariableResolver> getVariableResolverMaker() {
+        return variableResolverMaker;
+    }
+}

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/VariableResolverPlugin.java

@@ -0,0 +1,95 @@
+/*
+ * 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.jsr223;
+
+import org.apache.tinkerpop.gremlin.language.grammar.VariableResolver;
+
+import java.util.HashMap;
+import java.util.Map;
+import java.util.function.Function;
+
+/**
+ * A plugin that allows for the configuration of a {@link VariableResolver} implementation to be used by the
+ * {@link GremlinLangScriptEngine}. By default, it will use the {@link VariableResolver.DirectVariableResolver} which
+ * directly resolves variable name to a value from the binding in the script engine context. This is the most common
+ * usage relevant for most users and providers. Other options are reserved for more advanced use cases.
+ */
+public class VariableResolverPlugin extends AbstractGremlinPlugin {
+    private static final String NAME = "tinkerpop.variableResolver";
+
+    public static final Map<String, Function<Map<String, Object>, VariableResolver>> VARIABLE_RESOLVERS =
+            new HashMap<String, Function<Map<String, Object>, VariableResolver>>() {{
+                put(VariableResolver.DirectVariableResolver.class.getSimpleName(), VariableResolver.DirectVariableResolver::new);
+                put(VariableResolver.DefaultVariableResolver.class.getSimpleName(), VariableResolver.DefaultVariableResolver::new);
+                put(VariableResolver.NoVariableResolver.class.getSimpleName(), m -> VariableResolver.NoVariableResolver.instance());
+                put(VariableResolver.NullVariableResolver.class.getSimpleName(), m -> VariableResolver.NullVariableResolver.instance());
+    }};
+
+    private VariableResolverPlugin(final VariableResolverPlugin.Builder builder) {
+        super(NAME, new VariableResolverCustomizer(builder.variableResolverMaker));
+    }
+
+    /**
+     * Builds a set of static bindings.
+     */
+    public static VariableResolverPlugin.Builder build() {
+        return new VariableResolverPlugin.Builder();
+    }
+
+    public static final class Builder {
+
+        Function<Map<String,Object>, VariableResolver> variableResolverMaker = VariableResolver.DirectVariableResolver::new;
+
+        private Builder() {}
+
+        /**
+         * Sets the type of {@link VariableResolver} to use by specifying a simple class name associated with the
+         * inner classes in that interface or a fully qualified class name. The assumption is that implementations
+         * will allow a constructor that takes a {@code Map} which contains the bindings from the script engine context.
+         * Implementations are can then decide how to resolve variables in the script based on that {@code Map} or some
+         * other mechanism.
+         */
+        public VariableResolverPlugin.Builder resolver(final String resolverName) {
+            if (VARIABLE_RESOLVERS.containsKey(resolverName)) {
+                this.variableResolverMaker = VARIABLE_RESOLVERS.get(resolverName);
+            } else {
+                try {
+                    // Assuming resolverName is a fully qualified class name if it's not in the simple name map
+                    final Class<?> clazz = Class.forName(resolverName);
+                    if (VariableResolver.class.isAssignableFrom(clazz)) {
+                        this.variableResolverMaker = map -> {
+                            try {
+                                return (VariableResolver) clazz.getConstructor(Map.class).newInstance(map);
+                            } catch (Exception e) {
+                                throw new RuntimeException("Error instantiating VariableResolver", e);
+                            }
+                        };
+                    }
+                } catch (ClassNotFoundException e) {
+                    throw new RuntimeException("VariableResolver class not found: " + resolverName, e);
+                }
+            }
+            return this;
+        }
+
+        public VariableResolverPlugin create() {
+            return new VariableResolverPlugin(this);
+        }
+    }
+}

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/TraversalMethodVisitor.java

@@ -23,6 +23,8 @@
 import org.apache.tinkerpop.gremlin.process.traversal.Order;
 import org.apache.tinkerpop.gremlin.process.traversal.Traversal;
 import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
+import org.apache.tinkerpop.gremlin.process.traversal.step.GType;
+import org.apache.tinkerpop.gremlin.process.traversal.step.GValue;
 import org.apache.tinkerpop.gremlin.structure.VertexProperty.Cardinality;
 
 import java.util.LinkedHashMap;
@@ -445,7 +447,14 @@ public GraphTraversal visitTraversalMethod_combine_Object(final GremlinParser.Tr
      */
     @Override
     public GraphTraversal visitTraversalMethod_coin(final GremlinParser.TraversalMethod_coinContext ctx) {
-        return graphTraversal.coin(((Number) antlr.argumentVisitor.visitFloatArgument(ctx.floatArgument())).doubleValue());
+        final Object literalOrVar = antlr.argumentVisitor.visitFloatArgument(ctx.floatArgument());
+        if (literalOrVar instanceof Number)
+            return graphTraversal.coin(((Number) literalOrVar).doubleValue());
+        else if (literalOrVar instanceof GValue && ((GValue) literalOrVar).getType().isNumeric())
+            return graphTraversal.asAdmin().coin((GValue<Double>) literalOrVar);
+        else
+            throw new IllegalArgumentException("coin() argument must be a double");
+
     }
 
     /**

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/language/grammar/VariableResolver.java

@@ -18,24 +18,25 @@
  */
 package org.apache.tinkerpop.gremlin.language.grammar;
 
+import org.apache.tinkerpop.gremlin.process.traversal.step.GValue;
 import java.util.Map;
 import java.util.function.BiFunction;
 
 /**
  * Resolves parameters in Gremlin to objects.
  */
-public interface VariableResolver extends BiFunction<String, GremlinParser.VariableContext, Object> {
+public interface VariableResolver<T> extends BiFunction<String, GremlinParser.VariableContext, T> {
 
     /**
      * This function resolves a variable name and the given parsers context to an object.
      */
     @Override
-    Object apply(final String varName, final GremlinParser.VariableContext variableContext);
+    T apply(final String varName, final GremlinParser.VariableContext variableContext);
 
     /**
      * This {@link VariableResolver} implementation throws exceptions for all variable names.
      */
-    class NoVariableResolver implements VariableResolver {
+    class NoVariableResolver implements VariableResolver<Object> {
         private static NoVariableResolver instance = new NoVariableResolver();
 
         public static VariableResolver instance() {
@@ -48,12 +49,35 @@ public Object apply(final String s, final GremlinParser.VariableContext variable
         }
     }
 
+    /**
+     * Allows for a provided variable set in the form of a {@code Map}, where the keys are variable names and the
+     * values are the parameter values to be injected into the traversal in their place. The value is provided to a
+     * {@link GValue} object along with the variable name for further reference.
+     */
+    class DefaultVariableResolver implements VariableResolver<GValue<?>> {
+
+        private final Map<String, Object> variables;
+
+        public DefaultVariableResolver(final Map<String, Object> variables) {
+            this.variables = variables;
+        }
+
+        @Override
+        public GValue<?> apply(final String s, final GremlinParser.VariableContext variableContext) {
+            if (!variables.containsKey(s)) {
+                throw new VariableResolverException(String.format("No variable found for %s", s));
+            }
+
+            return GValue.of(s, variables.get(s));
+        }
+    }
+
     /**
      * This {@link VariableResolver} simply provides a {@code null} value for all variable names. It's typical use
      * is for when you really don't intend to execute the traversal and just want to get an instance of one when
      * bindings are being used as with {@link NoOpTerminalVisitor}.
      */
-    class NullVariableResolver implements VariableResolver {
+    class NullVariableResolver implements VariableResolver<Object> {
         private static NullVariableResolver instance = new NullVariableResolver();
 
         public static VariableResolver instance() {
@@ -70,11 +94,11 @@ public Object apply(final String s, final GremlinParser.VariableContext variable
      * Allows for a provided variable set in the form of a {@code Map}, where the keys are variable names and the
      * values are the parameter values to be injected into the traversal in their place.
      */
-    class DefaultVariableResolver implements VariableResolver {
+    class DirectVariableResolver implements VariableResolver<Object> {
 
         private final Map<String, Object> variables;
 
-        public DefaultVariableResolver(final Map<String, Object> variables) {
+        public DirectVariableResolver(final Map<String, Object> variables) {
             this.variables = variables;
         }
 
@@ -87,4 +111,5 @@ public Object apply(final String s, final GremlinParser.VariableContext variable
             return variables.get(s);
         }
     }
+
 }

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Bytecode.java

@@ -19,6 +19,7 @@
 
 package org.apache.tinkerpop.gremlin.process.traversal;
 
+import org.apache.tinkerpop.gremlin.process.traversal.step.GValue;
 import org.apache.tinkerpop.gremlin.process.traversal.strategy.TraversalStrategyProxy;
 import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
 import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
@@ -295,8 +296,13 @@ private final Object convertArgument(final Object argument, final boolean search
             if (null != variable)
                 return new Binding<>(variable, convertArgument(argument, false));
         }
-        //
-        if (argument instanceof Traversal) {
+
+        // a GValue gets converted to its value because we don't serialize it. it really doesn't have anything to do
+        // with bytecode/remoting. it's an internal construct related to parsing with the grammar that is leaking
+        // over here. in 4.x with bytecode removed, we won't need to worry about this.
+        if (argument instanceof GValue)
+            return convertArgument(((GValue) argument).get(), false);
+        else if (argument instanceof Traversal) {
             // prevent use of "g" to spawn child traversals
             if (((Traversal) argument).asAdmin().getTraversalSource().isPresent())
                 throw new IllegalStateException(String.format(

gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/P.java

@@ -18,6 +18,7 @@
  */
 package org.apache.tinkerpop.gremlin.process.traversal;
 
+import org.apache.tinkerpop.gremlin.process.traversal.step.GValue;
 import org.apache.tinkerpop.gremlin.process.traversal.util.AndP;
 import org.apache.tinkerpop.gremlin.process.traversal.util.OrP;
 
@@ -74,7 +75,10 @@ public void setValue(final V value) {
 
     @Override
     public boolean test(final V testValue) {
-        return this.biPredicate.test(testValue, this.value);
+        if (this.value instanceof GValue)
+            return this.biPredicate.test(testValue, ((GValue<V>) this.value).get());
+        else
+            return this.biPredicate.test(testValue, this.value);
     }
 
     @Override