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); + }); + } +}