From 6ee0e26435e90bae590cdaf545a6a744f67abb35 Mon Sep 17 00:00:00 2001 From: Ryan Tan Date: Fri, 8 Mar 2024 16:58:08 -0800 Subject: [PATCH] Allow aliased client to pass along options --- CHANGELOG.asciidoc | 1 + .../tinkerpop/gremlin/driver/Client.java | 9 +- .../gremlin/driver/RequestOptions.java | 27 ++++++ .../driver/remote/DriverRemoteConnection.java | 28 +----- .../remote/DriverRemoteConnectionTest.java | 5 +- .../ClientWithOptionsIntegrateTest.java | 87 +++++++++++++++++++ 6 files changed, 124 insertions(+), 33 deletions(-) create mode 100644 gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ClientWithOptionsIntegrateTest.java diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 38ded69065c..4fea866ff62 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -32,6 +32,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima * Fixed issue where server errors weren't being properly parsed when sending bytecode over HTTP. * Improved bulkset contains check for elements if all elements in bulkset are of the same type * Fixed bug in `EarlyLimitStrategy` which was too aggressive when promoting `limit()` before `map()`. +* Updated aliased client to pass along options via `with()` when submitting traversals. * Fixed bug in mid-traversal `mergeE()` where mutations in `sideEffect()` were being applied to the current traverser rather than a `onMatch` edge. [[release-3-6-6]] 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 513ec920962..415edde6644 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.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.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 static class AliasClusteredClient extends Client { @Override public CompletableFuture 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 ac923f99edf..cd5c2b68e03 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 static Builder build() { return new Builder(); } + public static RequestOptions getRequestOptions(final Bytecode bytecode) { + final Iterator itty = BytecodeHelper.findStrategies(bytecode, OptionsStrategy.class); + final Builder builder = RequestOptions.build(); + while (itty.hasNext()) { + final OptionsStrategy optionsStrategy = itty.next(); + final Map 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 aliases = null; private Map 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 13cc23dcf84..831c606e96c 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 @@ 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 @@ Optional getSessionId() { return Optional.empty(); } - protected static RequestOptions getRequestOptions(final Bytecode bytecode) { - final Iterator itty = BytecodeHelper.findStrategies(bytecode, OptionsStrategy.class); - final RequestOptions.Builder builder = RequestOptions.build(); - while (itty.hasNext()) { - final OptionsStrategy optionsStrategy = itty.next(); - final Map 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 71011331067..cbc0233eb72 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 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 void shouldBuildRequestOptions() { @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 00000000000..a120f999d4c --- /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 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 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 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 res = client.submit("ggrateful.with(\"evaluationTimeout\", 1).V().both().both().both();").all().get(); + fail("Failed to time out. Result: " + res); + }); + } +}