cleanup

apache · Jun 10, 2024 · 3174674 · 3174674
1 parent 1c46add
commit 3174674
Show file tree

Hide file tree

Showing 7 changed files with 58 additions and 45 deletions.
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Bytecode.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Bytecode.java
@@ -63,6 +63,7 @@ public class Bytecode implements Cloneable, Serializable {
     private StringBuilder gremlin = new StringBuilder();
     private Map<String, Object> parameters = new HashMap<>();
     private static final AtomicInteger paramCount = new AtomicInteger(0);
+    // [Discuss] probably ThreadLocal<Integer> is faster, but unsafe for multithreaded traversal construction.
     // private static final ThreadLocal<Integer> paramCount = ThreadLocal.withInitial(() -> 0);
     private final List<OptionsStrategy> optionsStrategies = new ArrayList<>();
 
@@ -279,50 +280,54 @@ public void addSource(final String sourceName, final Object... arguments) {
         Bindings.clear();
 
         if (sourceName.equals(TraversalSource.Symbols.withStrategies) && arguments.length != 0) {
-            final StringBuilder sb = new StringBuilder();
-            for (int i = 0; i < arguments.length; i++) {
-                // special handling for OptionsStrategy
-                if (arguments[i] instanceof OptionsStrategy) {
-                    optionsStrategies.add((OptionsStrategy) arguments[i]);
-                    break;
-                }
-
-                final Configuration configuration = ((TraversalStrategy) arguments[i]).getConfiguration();
-
-                if (configuration.isEmpty()) {
-                    sb.append(arguments[i].getClass().getSimpleName());
-                } else {
-                    sb.append("new ")
-                            .append(arguments[i].getClass().getSimpleName())
-                            .append("(");
-
-                    configuration.getKeys().forEachRemaining(key -> {
-                        if (!key.equals(STRATEGY)) {
-                            sb.append(key).append(":").append(argAsString(configuration.getProperty(key))).append(",");
-                        }
-                    });
-                    // remove last comma
-                    if (sb.lastIndexOf(",") == sb.length() - 1) {
-                        sb.setLength(sb.length() - 1);
-                    }
-
-                    sb.append(')');
-                }
-
-                if (i != arguments.length - 1)
-                    sb.append(',');
-            }
+            final String args =  buildStrategyArgs(arguments);
 
             // possible to have empty strategies list to send
-            if (sb.length() != 0) {
-                gremlin.append('.').append(TraversalSource.Symbols.withStrategies).append('(').append(sb).append(')');
+            if (!args.isEmpty()) {
+                gremlin.append('.').append(TraversalSource.Symbols.withStrategies).append('(').append(args).append(')');
             }
             return;
         }
 
+        addToGremlin(sourceName, arguments);
+    }
 
+    private String buildStrategyArgs(final Object[] arguments) {
+        final StringBuilder sb = new StringBuilder();
+        for (int i = 0; i < arguments.length; i++) {
+            // special handling for OptionsStrategy
+            if (arguments[i] instanceof OptionsStrategy) {
+                optionsStrategies.add((OptionsStrategy) arguments[i]);
+                break;
+            }
 
-        addToGremlin(sourceName, arguments);
+            final Configuration configuration = ((TraversalStrategy) arguments[i]).getConfiguration();
+
+            if (configuration.isEmpty()) {
+                sb.append(arguments[i].getClass().getSimpleName());
+            } else {
+                sb.append("new ")
+                        .append(arguments[i].getClass().getSimpleName())
+                        .append("(");
+
+                configuration.getKeys().forEachRemaining(key -> {
+                    if (!key.equals(STRATEGY)) {
+                        sb.append(key).append(":").append(argAsString(configuration.getProperty(key))).append(",");
+                    }
+                });
+                // remove last comma
+                if (sb.lastIndexOf(",") == sb.length() - 1) {
+                    sb.setLength(sb.length() - 1);
+                }
+
+                sb.append(')');
+            }
+
+            if (i != arguments.length - 1)
+                sb.append(',');
+        }
+
+        return sb.toString();
     }
 
     /**

diff --git a/...iver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java b/...iver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java
@@ -227,7 +227,7 @@ public static DriverRemoteConnection using(final Configuration conf) {
     public <E> CompletableFuture<RemoteTraversal<?, E>> submitAsync(final Bytecode gremlincode) throws RemoteConnectionException {
         try {
             gremlincode.addG(remoteTraversalSourceName);
-            return client.submitAsync(gremlincode.getGremlin(), gremlincode.getParameters())
+            return client.submitAsync(gremlincode.getGremlin(), getRequestOptions(gremlincode))
                     .thenApply(rs -> new DriverRemoteTraversal<>(rs, client, attachElements, conf));
         } catch (Exception ex) {
             throw new RemoteConnectionException(ex);
@@ -248,7 +248,7 @@ protected static RequestOptions getRequestOptions(final Bytecode bytecode) {
         final RequestOptions.Builder builder = RequestOptions.build();
         while (itty.hasNext()) {
             final OptionsStrategy optionsStrategy = itty.next();
-            final Map<String,Object> options = optionsStrategy.getOptions();
+            final Map<String, Object> options = optionsStrategy.getOptions();
             if (options.containsKey(ARGS_EVAL_TIMEOUT))
                 builder.timeout(((Number) options.get(ARGS_EVAL_TIMEOUT)).longValue());
             if (options.containsKey(ARGS_BATCH_SIZE))
@@ -258,6 +258,11 @@ protected static RequestOptions getRequestOptions(final Bytecode bytecode) {
             if (options.containsKey(ARGS_LANGUAGE))
                 builder.language((String) options.get(ARGS_LANGUAGE));
         }
+
+        final Map<String, Object> parameters = bytecode.getParameters();
+        if (parameters != null && !parameters.isEmpty()) {
+            parameters.forEach(builder::addParameter);
+        }
         return builder.create();
     }
 

diff --git a/...river/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversal.java b/...river/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteTraversal.java
@@ -114,7 +114,6 @@ public boolean hasNext() {
         @Override
         public Traverser.Admin<E> next() {
             return new DefaultRemoteTraverser<>((E)inner.next().getObject(), 1);
-            // return (RemoteTraverser<E>) inner.next().getObject(); // !!!
         }
     }
 

diff --git a/...src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java b/...src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
@@ -314,7 +314,6 @@ private void iterateScriptEvalResult(final Context context, MessageSerializerV4<
         }
 
         final Map<String, Object> args = message.getFields();
-        // !!!
         final String language = args.containsKey(TokensV4.ARGS_LANGUAGE) ? (String) args.get(TokensV4.ARGS_LANGUAGE) : "gremlin-groovy";
         final GremlinScriptEngine scriptEngine = gremlinExecutor.getScriptEngineManager().getEngineByName(language);
 

diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteWorld.java
@@ -161,6 +161,12 @@ public GraphTraversalSource getGraphTraversalSource(final LoadGraphWith.GraphDat
 
     public static class GraphBinaryRemoteWorld extends RemoteWorld {
         public GraphBinaryRemoteWorld() { super(createTestCluster(SerializersV4.GRAPHBINARY_V4)); }
+
+        @Override
+        public GraphTraversalSource getGraphTraversalSource(final LoadGraphWith.GraphData graphData) {
+            final GraphTraversalSource g = super.getGraphTraversalSource(graphData);
+            return g.with("language", "gremlin-lang");
+        }
     }
 
     ////////////////////////////////////////////////////////////////////////////////////////////////////////////////////

diff --git a/...lin-server/src/test/java/org/apache/tinkerpop/gremlin/server/HttpDriverIntegrateTest.java b/...lin-server/src/test/java/org/apache/tinkerpop/gremlin/server/HttpDriverIntegrateTest.java
@@ -183,7 +183,7 @@ public void shouldSubmitGremlinWithMergeV() {
     public void shouldHandleInfinity() {
         final Cluster cluster = TestClientFactory.build().create();
         try {
-            final GraphTraversalSource g = traversal().with(DriverRemoteConnection.using(cluster));
+            final GraphTraversalSource g = traversal().with(DriverRemoteConnection.using(cluster)).with("language", "gremlin-lang");
             final double result = g.inject(Double.POSITIVE_INFINITY).is(P.eq(Double.POSITIVE_INFINITY)).toList().get(0);
             assertEquals(result, Double.POSITIVE_INFINITY, 0.01);
         } catch (Exception ex) {
@@ -245,7 +245,7 @@ public void shouldDeserializeErrorWithGraphBinary() {
             g.V().next();
             fail("Expected exception to be thrown.");
         } catch (Exception ex) {
-            assert ex.getMessage().contains("The traversal source [doesNotExist] for alias [g] is not configured on the server.");
+            assert ex.getMessage().contains("Could not alias [g] to [doesNotExist] as [doesNotExist] not in the Graph or TraversalSource global bindings");
         } finally {
             cluster.close();
         }

diff --git a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4.java b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4.java
@@ -51,9 +51,8 @@ private RequestMessageV4(final Object gremlin, final Map<String, Object> fields)
 
         if (gremlin instanceof String) {
             gremlinType = TokensV4.OPS_EVAL;
-            // !!!
-//             this.fields.putIfAbsent(TokensV4.ARGS_LANGUAGE, "gremlin-groovy");
-            this.fields.putIfAbsent(TokensV4.ARGS_LANGUAGE, "gremlin-lang");
+            // default language is "gremlin-groovy" for now, will be replaced in following PR's
+            this.fields.putIfAbsent(TokensV4.ARGS_LANGUAGE, "gremlin-groovy");
         } else if (gremlin instanceof Bytecode) {
             gremlinType = TokensV4.OPS_BYTECODE;
         } else {