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

colegreer pushed a commit to branch 3.6-dev
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


The following commit(s) were added to refs/heads/3.6-dev by this push:
     new 7776458585 Allow aliased client to pass along options (#2524)
7776458585 is described below

commit 777645858590475b8d612d6c1d4cdf63443cde3b
Author: Ryan Tan <65996005+r...@users.noreply.github.com>
AuthorDate: Mon Mar 25 15:16:59 2024 -0700

    Allow aliased client to pass along options (#2524)
    
    Refactored DriverRemoteConnection.getRequestOptions() to be a public static 
method in RequestOptions so that Client.submitAsync() can get and pass along 
options with it. This fixes the issue where aliased clients submitting 
traversals containing with() configurations would ignore sending these options 
as part of the request in https://issues.apache.org/jira/browse/TINKERPOP-3039.
---
 CHANGELOG.asciidoc                                 |  1 +
 .../apache/tinkerpop/gremlin/driver/Client.java    |  9 ++-
 .../tinkerpop/gremlin/driver/RequestOptions.java   | 27 +++++++
 .../driver/remote/DriverRemoteConnection.java      | 28 +------
 .../driver/remote/DriverRemoteConnectionTest.java  |  5 +-
 .../server/ClientWithOptionsIntegrateTest.java     | 87 ++++++++++++++++++++++
 6 files changed, 124 insertions(+), 33 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index eceb847c17..a41951c2c1 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -37,6 +37,7 @@ 
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
 * Fixed bug in `EarlyLimitStrategy` which was too aggressive when promoting 
`limit()` before `map()`.
 * Prevented mid-traversal `mergeE()` and `mergeV()` from operating on an 
incoming `Traverser` that contains an `Element`.
 * Improved performance of the application of `FilterRankingStrategy` for large 
traversals with deeply nested traversals by improving the cache operation.
+* Updated aliased client to pass along options via `with()` when submitting 
traversals.
 
 [[release-3-6-6]]
 === TinkerPop 3.6.6 (November 20, 2023)
diff --git 
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java 
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java
index 513ec92096..415edde664 100644
--- 
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java
+++ 
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/Client.java
@@ -19,8 +19,6 @@
 package org.apache.tinkerpop.gremlin.driver;
 
 import org.apache.commons.lang3.exception.ExceptionUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
 import org.apache.tinkerpop.gremlin.driver.exception.ConnectionException;
 import org.apache.tinkerpop.gremlin.driver.exception.NoHostAvailableException;
 import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
@@ -30,6 +28,8 @@ import 
org.apache.tinkerpop.gremlin.process.traversal.TraversalSource;
 import org.apache.tinkerpop.gremlin.process.traversal.Traverser;
 import org.apache.tinkerpop.gremlin.structure.Graph;
 import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.net.ssl.SSLException;
 import java.net.ConnectException;
@@ -51,7 +51,8 @@ import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.Consumer;
 import java.util.stream.Collectors;
-import java.util.stream.IntStream;
+
+import static 
org.apache.tinkerpop.gremlin.driver.RequestOptions.getRequestOptions;
 
 /**
  * A {@code Client} is constructed from a {@link Cluster} and represents a way 
to send messages to Gremlin Server.
@@ -642,7 +643,7 @@ public abstract class Client {
 
         @Override
         public CompletableFuture<ResultSet> submitAsync(final Bytecode 
bytecode) {
-            return submitAsync(bytecode, RequestOptions.EMPTY);
+            return submitAsync(bytecode, getRequestOptions(bytecode));
         }
 
         @Override
diff --git 
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/RequestOptions.java
 
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/RequestOptions.java
index ac923f99ed..cd5c2b68e0 100644
--- 
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/RequestOptions.java
+++ 
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/RequestOptions.java
@@ -19,12 +19,21 @@
 package org.apache.tinkerpop.gremlin.driver;
 
 import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
+import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
+import 
org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy;
+import org.apache.tinkerpop.gremlin.process.traversal.util.BytecodeHelper;
 
 import java.util.HashMap;
+import java.util.Iterator;
 import java.util.Map;
 import java.util.Optional;
 import java.util.UUID;
 
+import static org.apache.tinkerpop.gremlin.driver.Tokens.ARGS_BATCH_SIZE;
+import static org.apache.tinkerpop.gremlin.driver.Tokens.ARGS_EVAL_TIMEOUT;
+import static org.apache.tinkerpop.gremlin.driver.Tokens.ARGS_USER_AGENT;
+import static org.apache.tinkerpop.gremlin.driver.Tokens.REQUEST_ID;
+
 /**
  * Options that can be supplied on a per request basis.
  *
@@ -84,6 +93,24 @@ public final class RequestOptions {
         return new Builder();
     }
 
+    public static RequestOptions getRequestOptions(final Bytecode bytecode) {
+        final Iterator<OptionsStrategy> itty = 
BytecodeHelper.findStrategies(bytecode, OptionsStrategy.class);
+        final Builder builder = RequestOptions.build();
+        while (itty.hasNext()) {
+            final OptionsStrategy optionsStrategy = itty.next();
+            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(REQUEST_ID))
+                builder.overrideRequestId((UUID) options.get(REQUEST_ID));
+            if (options.containsKey(ARGS_BATCH_SIZE))
+                builder.batchSize(((Number) 
options.get(ARGS_BATCH_SIZE)).intValue());
+            if (options.containsKey(ARGS_USER_AGENT))
+                builder.userAgent((String) options.get(ARGS_USER_AGENT));
+        }
+        return builder.create();
+    }
+
     public static final class Builder {
         private Map<String,String> aliases = null;
         private Map<String, Object> parameters = null;
diff --git 
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java
 
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java
index 13cc23dcf8..831c606e96 100644
--- 
a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java
+++ 
b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnection.java
@@ -21,27 +21,19 @@ package org.apache.tinkerpop.gremlin.driver.remote;
 import org.apache.commons.configuration2.Configuration;
 import org.apache.tinkerpop.gremlin.driver.Client;
 import org.apache.tinkerpop.gremlin.driver.Cluster;
-import org.apache.tinkerpop.gremlin.driver.RequestOptions;
 import org.apache.tinkerpop.gremlin.process.remote.RemoteConnection;
 import org.apache.tinkerpop.gremlin.process.remote.RemoteConnectionException;
 import org.apache.tinkerpop.gremlin.process.remote.traversal.RemoteTraversal;
 import org.apache.tinkerpop.gremlin.process.traversal.Bytecode;
 import 
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
-import 
org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy;
-import org.apache.tinkerpop.gremlin.process.traversal.util.BytecodeHelper;
 import org.apache.tinkerpop.gremlin.structure.Transaction;
 import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
 
-import java.util.Iterator;
-import java.util.Map;
 import java.util.Optional;
 import java.util.UUID;
 import java.util.concurrent.CompletableFuture;
 
-import static org.apache.tinkerpop.gremlin.driver.Tokens.ARGS_BATCH_SIZE;
-import static org.apache.tinkerpop.gremlin.driver.Tokens.ARGS_EVAL_TIMEOUT;
-import static org.apache.tinkerpop.gremlin.driver.Tokens.ARGS_USER_AGENT;
-import static org.apache.tinkerpop.gremlin.driver.Tokens.REQUEST_ID;
+import static 
org.apache.tinkerpop.gremlin.driver.RequestOptions.getRequestOptions;
 
 /**
  * A {@link RemoteConnection} implementation for Gremlin Server. Each {@code 
DriverServerConnection} is bound to one
@@ -244,24 +236,6 @@ public class DriverRemoteConnection implements 
RemoteConnection {
         return Optional.empty();
     }
 
-    protected static RequestOptions getRequestOptions(final Bytecode bytecode) 
{
-        final Iterator<OptionsStrategy> itty = 
BytecodeHelper.findStrategies(bytecode, OptionsStrategy.class);
-        final RequestOptions.Builder builder = RequestOptions.build();
-        while (itty.hasNext()) {
-            final OptionsStrategy optionsStrategy = itty.next();
-            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(REQUEST_ID))
-                builder.overrideRequestId((UUID) options.get(REQUEST_ID));
-            if (options.containsKey(ARGS_BATCH_SIZE))
-                builder.batchSize(((Number) 
options.get(ARGS_BATCH_SIZE)).intValue());
-            if (options.containsKey(ARGS_USER_AGENT))
-                builder.userAgent((String) options.get(ARGS_USER_AGENT));
-        }
-        return builder.create();
-    }
-
     @Override
     public void close() throws Exception {
         try {
diff --git 
a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnectionTest.java
 
b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnectionTest.java
index 7101133106..cbc0233eb7 100644
--- 
a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnectionTest.java
+++ 
b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/DriverRemoteConnectionTest.java
@@ -26,6 +26,7 @@ import org.junit.Test;
 
 import java.util.UUID;
 
+import static 
org.apache.tinkerpop.gremlin.driver.RequestOptions.getRequestOptions;
 import static org.junit.Assert.assertEquals;
 
 /**
@@ -37,7 +38,7 @@ public class DriverRemoteConnectionTest {
     @Test
     public void shouldBuildRequestOptions() {
         final UUID requestId = 
UUID.fromString("34a9f45f-8854-4d33-8b40-92a8171ee495");
-        final RequestOptions options = 
DriverRemoteConnection.getRequestOptions(
+        final RequestOptions options = getRequestOptions(
                 g.with("x").
                         with("y", 100).
                         with(Tokens.ARGS_BATCH_SIZE, 1000).
@@ -53,7 +54,7 @@ public class DriverRemoteConnectionTest {
 
     @Test
     public void shouldBuildRequestOptionsWithNumerics() {
-        final RequestOptions options = 
DriverRemoteConnection.getRequestOptions(
+        final RequestOptions options = getRequestOptions(
                 g.with(Tokens.ARGS_BATCH_SIZE, 100).
                   with(Tokens.ARGS_EVAL_TIMEOUT, 1000).
                   V().asAdmin().getBytecode());
diff --git 
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ClientWithOptionsIntegrateTest.java
 
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ClientWithOptionsIntegrateTest.java
new file mode 100644
index 0000000000..a120f999d4
--- /dev/null
+++ 
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ClientWithOptionsIntegrateTest.java
@@ -0,0 +1,87 @@
+/*
+ * 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.server;
+
+import org.apache.tinkerpop.gremlin.driver.Client;
+import org.apache.tinkerpop.gremlin.driver.Cluster;
+import org.apache.tinkerpop.gremlin.driver.Result;
+import org.apache.tinkerpop.gremlin.driver.remote.DriverRemoteConnection;
+import org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversal;
+import 
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
+import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.junit.Test;
+
+import java.util.List;
+import java.util.concurrent.CompletionException;
+import java.util.concurrent.ExecutionException;
+
+import static 
org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource.traversal;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.fail;
+
+/**
+ * Tests for options sent by the client using with().
+ *
+ * @author Ryan Tan
+ */
+public class ClientWithOptionsIntegrateTest extends 
AbstractGremlinServerIntegrationTest {
+
+    @Test
+    public void shouldTimeOutAliasedClientSendingBytecode() {
+        final Cluster cluster = TestClientFactory.build().create();
+        final Client client = cluster.connect().alias("ggrateful");
+        final GraphTraversalSource g = 
traversal().withRemote(DriverRemoteConnection.using(client, "g"));
+        final GraphTraversal traversal = g.with("evaluationTimeout", 
1).V().both().both().both();
+        assertThrows(ExecutionException.class, () -> {
+            final List<Result> res = client.submit(traversal).all().get();
+            fail("Failed to time out. Result: " + res);
+        });
+    }
+
+    @Test
+    public void shouldTimeOutnonAliasedClientSendingByteCode() {
+        final Cluster cluster = TestClientFactory.build().create();
+        final Client client = cluster.connect();
+        final GraphTraversalSource g = 
traversal().withRemote(DriverRemoteConnection.using(client, "ggrateful"));
+        assertThrows(CompletionException.class, () -> {
+            final List<Vertex> res = g.with("evaluationTimeout", 
1).V().both().both().both().toList();
+            fail("Failed to time out. Result: " + res);
+        });
+    }
+
+    @Test
+    public void shouldTimeOutAliasedClientSubmittingScript() {
+        final Cluster cluster = TestClientFactory.build().create();
+        final Client client = cluster.connect().alias("ggrateful");
+        assertThrows(ExecutionException.class, () -> {
+            final List<Result> res = 
client.submit("g.with(\"evaluationTimeout\", 
1).V().both().both().both();").all().get();
+            fail("Failed to time out. Result: " + res);
+        });
+    }
+
+    @Test
+    public void shouldTimeOutNonAliasedClientSubmittingScript() {
+        final Cluster cluster = TestClientFactory.build().create();
+        final Client client = cluster.connect();
+        assertThrows(ExecutionException.class, () -> {
+            final List<Result> res = 
client.submit("ggrateful.with(\"evaluationTimeout\", 
1).V().both().both().both();").all().get();
+            fail("Failed to time out. Result: " + res);
+        });
+    }
+}

Reply via email to