This is an automated email from the ASF dual-hosted git repository.
kenhuuu pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
The following commit(s) were added to refs/heads/master by this push:
new 4057e3356e Honor per-request options embedded in a script via with
options (#3518)
4057e3356e is described below
commit 4057e3356e0b54816a0b8c047e62d4f61c37941e
Author: Ken Hu <[email protected]>
AuthorDate: Fri Jul 17 14:06:19 2026 -0700
Honor per-request options embedded in a script via with options (#3518)
The server now applies per-request options set inside a submitted Gremlin
string's with(): evaluationTimeout/timeoutMillis, materializeProperties,
batchSize, bulkResults, and language, with a single precedence:
script embedded with() > request field > (header, bulkResults only) >
default.
This fixes a regression where a script embedded timeoutMillis was silently
ignored on the raw string path.
Options are resolved once in Context; batchSize is parsed and validated
there,
returning 400 Bad Request for a non positive or out of range value rather
than
stalling result iteration or surfacing an uncaught 500. Validation runs
before
any transaction begin side effects. g, parameters, and transactionId remain
request field/header only, and the obsolete requestId scrape is removed from
GremlinScriptChecker.
Assisted-by: Claude Code:claude-opus-4-8
---
CHANGELOG.asciidoc | 1 +
docs/src/dev/provider/index.asciidoc | 12 +-
.../gremlin/jsr223/GremlinScriptChecker.java | 186 +++++++++---------
.../gremlin/jsr223/GremlinScriptCheckerTest.java | 208 ++++++++++++++-------
.../apache/tinkerpop/gremlin/server/Context.java | 111 +++++++++--
.../server/handler/HttpGremlinEndpointHandler.java | 45 +++--
.../gremlin/server/util/GremlinError.java | 6 +
.../tinkerpop/gremlin/server/ContextTest.java | 145 +++++++++++++-
.../server/GremlinServerHttpIntegrateTest.java | 139 ++++++++++++++
.../jsr223/GremlinScriptCheckerBenchmark.java | 6 +-
10 files changed, 645 insertions(+), 214 deletions(-)
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 9c80137a93..d0fb4b07b9 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -106,6 +106,7 @@
image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
* Renamed the request `bindings` field to `parameters` across the HTTP request
body, GraphBinary, and GraphSON, standardizing terminology for query parameters.
* Modified HTTP API to expect gremlin-lang strings for parameters and update
all GLVs to send requests in new format.
* Added string parameter parsing to `GremlinServer` to prevent traversal
injection and excessive nesting depths.
+* Modified how the server honors per-request options embedded in a script via
`with()`, extending `GremlinScriptChecker` to recognize `language`,
`batchSize`, and `bulkResults` (in addition to `timeoutMillis` and
`materializeProperties`) with uniform precedence.
* Modified all GLVs to detect unsupported types in `GremlinLang` and throw
consistent error for that case.
* Extended `GValue` parameterization to all GLVs (`gremlin-python`,
`gremlin-go`, `gremlin-javascript`, `gremlin-dotnet`), allowing parameters to
be passed directly in traversals as in Java.
* Relaxed `GValue` name validation by removing the reserved leading underscore
restriction.
diff --git a/docs/src/dev/provider/index.asciidoc
b/docs/src/dev/provider/index.asciidoc
index 773d5d6221..f2f505c368 100644
--- a/docs/src/dev/provider/index.asciidoc
+++ b/docs/src/dev/provider/index.asciidoc
@@ -1261,11 +1261,21 @@ the serializer specified by the `Content-Type` header.
The following are the key
|parameters |A gremlin-lang string that encodes a map of key/value pairs used
during query execution. Its usage depends on "language". For "gremlin-groovy",
these are applied as script variable bindings. For "gremlin-lang", these are
the query parameters. |String containing a gremlin-lang map literal |No
|g |The name of the graph traversal source to which the query applies.
Default: "g" |String containing traversal source name |No
|language |The name of the ScriptEngine to use to parse the gremlin query.
Default: "gremlin-lang" |String containing ScriptEngine name |No
+|batchSize |The number of results to include in each response chunk,
overriding the server configured `resultIterationBatchSize`. |Number between 1
and 2^31-1 |No
|materializeProperties |Whether to include all properties for results. One of
"tokens" or "all". |String |No
|bulkResults |Whether the results should be bulked by the server (only applies
to GraphBinary) |Boolean |No
|transactionId |A server-generated UUID that identifies an active transaction.
Must be included in all non-begin requests within that transaction. Omit for
non-transactional requests and for the initial begin request. |String |No
|=========================================================
+Several of these options may alternatively be supplied inside the Gremlin
query itself using the `with()` source step
+(for example `g.with('timeoutMillis', 500).V()`) when submitting a script. The
server recognizes the following options
+this way: `timeoutMillis`, `materializeProperties`, `batchSize`,
`bulkResults`, and `language`. When the same option is
+supplied both in the query via `with()` and as a request field, the value in
the query takes precedence. The full
+precedence order is: value embedded in the query via `with()`, then the
request field, then the server/connection
+default. The `parameters`, `transactionId`, `g`, and `gremlin` keys cannot be
set through `with()` and must be supplied
+as request fields; `g` is excluded because of the complex interactions between
different script engines and
+transactions.
+
==== HTTP Response
When Gremlin Server receives that request, it will decode it given the "mime
type", and execute it using the
@@ -2033,4 +2043,4 @@ in use with the Gremlin Console plugin host. Simply
instantiate and return a `R
include::gremlin-semantics.asciidoc[]
-include::policies.asciidoc[]
\ No newline at end of file
+include::policies.asciidoc[]
diff --git
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptChecker.java
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptChecker.java
index 1879669f26..2dd75c4c19 100644
---
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptChecker.java
+++
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptChecker.java
@@ -36,14 +36,16 @@ public class GremlinScriptChecker {
/**
* An empty result whose properties return as empty.
*/
- public static final Result EMPTY_RESULT = new Result(null, null, null);
+ public static final Result EMPTY_RESULT = new Result(null, null, null,
null, null);
/**
* At least one of these tokens should be present somewhere in the Gremlin
string for {@link #parse(String)} to
* take any action at all.
*/
private static final Set<String> tokens = new
HashSet<>(Arrays.asList("timeoutMillis", "TIMEOUT_MILLIS",
- "requestId", "REQUEST_ID", "materializeProperties",
"ARGS_MATERIALIZE_PROPERTIES"));
+ "materializeProperties", "ARGS_MATERIALIZE_PROPERTIES",
+ "language", "ARGS_LANGUAGE", "batchSize", "ARGS_BATCH_SIZE",
+ "bulkResults", "BULK_RESULTS"));
/**
* Matches single line comments, multi-line comments and space characters.
@@ -85,100 +87,51 @@ public class GremlinScriptChecker {
private static final String timeoutTokens =
"[\"']timeoutMillis[\"']|(?:Tokens\\.)?TIMEOUT_MILLIS";
/**
- * Regex fragment for the timeout tokens to look for. There are basically
four:
+ * Regex fragment for the materializeProperties to look for. There are
basically four:
* <ul>
- * <li>{@code requestId} which is a string value and thus single or
double quoted</li>
- * <li>{@code REQUEST_ID} which is a enum type of value which can be
referenced with or without a {@code Tokens} qualifier</li>
+ * <li>{@code materializeProperties} which is a string value and thus
single or double quoted</li>
+ * <li>{@code ARGS_MATERIALIZE_PROPERTIES} which is a enum type of
value which can be referenced with or without a {@code Tokens} qualifier</li>
* </ul>
- * See {@link #patternWithOptions} for a full explain as this regex is
embedded in there.
*/
- private static final String requestIdTokens =
"[\"']requestId[\"']|(?:Tokens\\.)?REQUEST_ID";
+ private static final String materializePropertiesTokens =
"[\"']materializeProperties[\"']|(?:Tokens\\.)?ARGS_MATERIALIZE_PROPERTIES";
/**
- * Regex fragment for the materializeProperties to look for. There are
basically four:
+ * Regex fragment for the {@code language} tokens to look for:
* <ul>
- * <li>{@code materializeProperties} which is a string value and thus
single or double quoted</li>
- * <li>{@code ARGS_MATERIALIZE_PROPERTIES} which is a enum type of
value which can be referenced with or without a {@code Tokens} qualifier</li>
+ * <li>{@code language} which is a string value and thus single or
double quoted</li>
+ * <li>{@code ARGS_LANGUAGE} which can be referenced with or without a
{@code Tokens} qualifier</li>
* </ul>
*/
- private static final String materializePropertiesTokens =
"[\"']materializeProperties[\"']|(?:Tokens\\.)?ARGS_MATERIALIZE_PROPERTIES";
+ private static final String languageTokens =
"[\"']language[\"']|(?:Tokens\\.)?ARGS_LANGUAGE";
/**
- * Matches {@code .with({timeout-token},{timeout})} with a matching group
on the {@code timeout}.
- * input: g.with('materializeProperties',100)
- * <pre>
- * From https://regex101.com/
- *
- *
\.with\((((?:["']materializeProperties["']|["']scriptEvaluationTimeout["']|(?:Tokens\.)?ARGS_EVAL_TIMEOUT|(?:Tokens\.)?ARGS_SCRIPT_EVAL_TIMEOUT),(?<to>\d*)(:?L|l)?)|((?:["']materializeProperties["']|(?:Tokens\.)?ARGS_MATERIALIZE_PROPERTIES),["'](?<mp>.*?)["']?)|((?:["']requestId["']|(?:Tokens\.)?REQUEST_ID),["'](?<rid>.*?)["']))\)
- *
- * gm
- * \. matches the character . with index 4610 (2E16 or 568) literally
(case sensitive)
- * with matches the characters with literally (case sensitive)
- * \( matches the character ( with index 4010 (2816 or 508) literally
(case sensitive)
- * 1st Capturing Group
(((?:["']materializeProperties["']|["']scriptEvaluationTimeout["']|(?:Tokens\.)?ARGS_EVAL_TIMEOUT|(?:Tokens\.)?ARGS_SCRIPT_EVAL_TIMEOUT),(?<to>\d*)(:?L|l)?)|((?:["']materializeProperties["']|(?:Tokens\.)?ARGS_MATERIALIZE_PROPERTIES),["'](?<mp>.*?)["']?)|((?:["']requestId["']|(?:Tokens\.)?REQUEST_ID),["'](?<rid>.*?)["']))
- * 1st Alternative
((?:["']materializeProperties["']|["']scriptEvaluationTimeout["']|(?:Tokens\.)?ARGS_EVAL_TIMEOUT|(?:Tokens\.)?ARGS_SCRIPT_EVAL_TIMEOUT),(?<to>\d*)(:?L|l)?)
- * 2nd Capturing Group
((?:["']materializeProperties["']|["']scriptEvaluationTimeout["']|(?:Tokens\.)?ARGS_EVAL_TIMEOUT|(?:Tokens\.)?ARGS_SCRIPT_EVAL_TIMEOUT),(?<to>\d*)(:?L|l)?)
- * Non-capturing group
(?:["']materializeProperties["']|["']scriptEvaluationTimeout["']|(?:Tokens\.)?ARGS_EVAL_TIMEOUT|(?:Tokens\.)?ARGS_SCRIPT_EVAL_TIMEOUT)
- * 1st Alternative ["']materializeProperties["']
- * Match a single character present in the list below ["']
- * "' matches a single character in the list "' (case sensitive)
- * materializeProperties
- * matches the characters materializeProperties literally (case sensitive)
- * Match a single character present in the list below ["']
- * "' matches a single character in the list "' (case sensitive)
- * 2nd Alternative ["']scriptEvaluationTimeout["']
- * Match a single character present in the list below ["']
- * "' matches a single character in the list "' (case sensitive)
- * scriptEvaluationTimeout matches the characters scriptEvaluationTimeout
literally (case sensitive)
- * Match a single character present in the list below ["']
- * "' matches a single character in the list "' (case sensitive)
- * 3rd Alternative (?:Tokens\.)?ARGS_EVAL_TIMEOUT
- * Non-capturing group (?:Tokens\.)?
- * ? matches the previous token between zero and one times, as many times
as possible, giving back as needed (greedy)
- * Tokens matches the characters Tokens literally (case sensitive)
- * \. matches the character . with index 4610 (2E16 or 568) literally
(case sensitive)
- * ARGS_EVAL_TIMEOUT matches the characters ARGS_EVAL_TIMEOUT literally
(case sensitive)
- * 4th Alternative (?:Tokens\.)?ARGS_SCRIPT_EVAL_TIMEOUT
- * Non-capturing group (?:Tokens\.)?
- * ? matches the previous token between zero and one times, as many times
as possible, giving back as needed (greedy)
- * Tokens matches the characters Tokens literally (case sensitive)
- * \. matches the character . with index 4610 (2E16 or 568) literally
(case sensitive)
- * ARGS_SCRIPT_EVAL_TIMEOUT matches the characters
ARGS_SCRIPT_EVAL_TIMEOUT literally (case sensitive)
- * , matches the character , with index 4410 (2C16 or 548) literally (case
sensitive)
- * Named Capture Group to (?<to>\d*)
- * \d matches a digit (equivalent to [0-9])
- * * matches the previous token between zero and unlimited times, as many
times as possible, giving back as needed (greedy)
- * 4th Capturing Group (:?L|l)?
- * ? matches the previous token between zero and one times, as many times
as possible, giving back as needed (greedy)
- * 1st Alternative :?L
- * : matches the character : with index 5810 (3A16 or 728) literally (case
sensitive)
- * ? matches the previous token between zero and one times, as many times
as possible, giving back as needed (greedy)
- * L matches the character L with index 7610 (4C16 or 1148) literally
(case sensitive)
- * 2nd Alternative l
- * l matches the character l with index 10810 (6C16 or 1548) literally
(case sensitive)
- * 2nd Alternative
((?:["']materializeProperties["']|(?:Tokens\.)?ARGS_MATERIALIZE_PROPERTIES),["'](?<mp>.*?)["']?)
- * 5th Capturing Group
((?:["']materializeProperties["']|(?:Tokens\.)?ARGS_MATERIALIZE_PROPERTIES),["'](?<mp>.*?)["']?)
- * Non-capturing group
(?:["']materializeProperties["']|(?:Tokens\.)?ARGS_MATERIALIZE_PROPERTIES)
- * 1st Alternative ["']materializeProperties["']
- * 2nd Alternative (?:Tokens\.)?ARGS_MATERIALIZE_PROPERTIES
- * , matches the character , with index 4410 (2C16 or 548) literally (case
sensitive)
- * Match a single character present in the list below ["']
- * "' matches a single character in the list "' (case sensitive)
- * Named Capture Group mp (?<mp>.*?)
- * Match a single character present in the list below ["']
- * 3rd Alternative
((?:["']requestId["']|(?:Tokens\.)?REQUEST_ID),["'](?<rid>.*?)["'])
- * 7th Capturing Group
((?:["']requestId["']|(?:Tokens\.)?REQUEST_ID),["'](?<rid>.*?)["'])
- * \) matches the character ) with index 4110 (2916 or 518) literally
(case sensitive)
- * Global pattern flags
- * g modifier: global. All matches (don't return after first match)
- * m modifier: multi line. Causes ^ and $ to match the begin/end of each
line (not only begin/end of string)
- * </pre>
+ * Regex fragment for the {@code batchSize} tokens to look for:
+ * <ul>
+ * <li>{@code batchSize} which is a string value and thus single or
double quoted</li>
+ * <li>{@code ARGS_BATCH_SIZE} which can be referenced with or without
a {@code Tokens} qualifier</li>
+ * </ul>
+ */
+ private static final String batchSizeTokens =
"[\"']batchSize[\"']|(?:Tokens\\.)?ARGS_BATCH_SIZE";
+
+ /**
+ * Regex fragment for the {@code bulkResults} tokens to look for:
+ * <ul>
+ * <li>{@code bulkResults} which is a string value and thus single or
double quoted</li>
+ * <li>{@code BULK_RESULTS} which can be referenced with or without a
{@code Tokens} qualifier</li>
+ * </ul>
+ */
+ private static final String bulkResultsTokens =
"[\"']bulkResults[\"']|(?:Tokens\\.)?BULK_RESULTS";
+
+ /**
+ * Matches supported traversal source request options supplied via {@code
with()} and captures their values.
*/
private static final Pattern patternWithOptions =
Pattern.compile("\\.with\\((((?:"
+ timeoutTokens + "),(?<to>\\d*)(:?L|l)?)|((?:"
+ materializePropertiesTokens +
"),[\"'](?<mp>.*?)[\"']?)|((?:"
- + requestIdTokens + "),[\"'](?<rid>.*?)[\"']))\\)");
+ + languageTokens + "),[\"'](?<lang>.*?)[\"'])|((?:"
+ + batchSizeTokens + "),(?<bs>-?\\d+))|((?:"
+ + bulkResultsTokens + "),(?<br>(?i:true|false))))\\)");
/**
* Parses a Gremlin script and extracts a {@code Result} containing
properties that are relevant to the checker.
@@ -199,8 +152,10 @@ public class GremlinScriptChecker {
// arguments given to Result class as null mean they weren't assigned
(or the parser didn't find them somehow - eek!)
Long timeout = null;
- String requestId = null;
String materializeProperties = null;
+ String language = null;
+ String batchSize = null;
+ Boolean bulkResults = null;
do {
// timeout is added up across all scripts
final String to = m.group("to");
@@ -209,16 +164,26 @@ public class GremlinScriptChecker {
timeout += Long.parseLong(to);
}
- // request id just uses the last one found
- final String rid = m.group("rid");
- if (rid != null) requestId = rid;
-
//materializeProperties just uses the last one found
final String mp = m.group("mp");
if (mp != null) materializeProperties = mp;
+
+ // language just uses the last one found
+ final String lang = m.group("lang");
+ if (lang != null) language = lang;
+
+ // batchSize just uses the last one found. it is captured as the
raw string (not parsed to a number) so
+ // that an out-of-range value does not throw here in the Context
constructor - the server parses and
+ // validates it where a bad value can be surfaced as a bad request
rather than an uncaught error.
+ final String bs = m.group("bs");
+ if (bs != null) batchSize = bs;
+
+ // bulkResults just uses the last one found
+ final String br = m.group("br");
+ if (br != null) bulkResults = Boolean.parseBoolean(br);
} while (m.find());
- return new Result(timeout, requestId, materializeProperties);
+ return new Result(timeout, materializeProperties, language, batchSize,
bulkResults);
}
/**
@@ -226,13 +191,18 @@ public class GremlinScriptChecker {
*/
public static class Result {
private final Long timeout;
- private final String requestId;
private final String materializeProperties;
+ private final String language;
+ private final String batchSize;
+ private final Boolean bulkResults;
- private Result(final Long timeout, final String requestId, final
String materializeProperties) {
+ private Result(final Long timeout, final String materializeProperties,
+ final String language, final String batchSize, final
Boolean bulkResults) {
this.timeout = timeout;
- this.requestId = requestId;
this.materializeProperties = materializeProperties;
+ this.language = language;
+ this.batchSize = batchSize;
+ this.bulkResults = bulkResults;
}
/**
@@ -244,27 +214,47 @@ public class GremlinScriptChecker {
}
/**
- * Gets the value of the request identifier supplied using the {@link
GraphTraversal#with(String, Object)} source step.
- * If there are multiple commands using this step, the last usage
should represent the id returned here.
+ * Gets the value of the materializeProperties supplied using the
{@link GraphTraversal#with(String, Object)} source step.
+ * If there are multiple commands using this step, the last usage
should represent the value returned here.
*/
- public Optional<String> getRequestId() {
- return null == requestId ? Optional.empty() :
Optional.of(requestId);
+ public Optional<String> getMaterializeProperties() {
+ return null == materializeProperties ? Optional.empty() :
Optional.of(materializeProperties);
}
/**
- * Gets the value of the materializeProperties supplied using the
{@link GraphTraversal#with(String, Object)} source step.
+ * Gets the value of the language supplied using the {@link
GraphTraversal#with(String, Object)} source step.
* If there are multiple commands using this step, the last usage
should represent the value returned here.
*/
- public Optional<String> getMaterializeProperties() {
- return null == materializeProperties ? Optional.empty() :
Optional.of(materializeProperties);
+ public Optional<String> getLanguage() {
+ return null == language ? Optional.empty() : Optional.of(language);
+ }
+
+ /**
+ * Gets the raw, unparsed value of the batchSize supplied using the
{@link GraphTraversal#with(String, Object)}
+ * source step. It is returned as the raw string (rather than a parsed
number) so that parsing and range
+ * validation happen where an invalid value can be surfaced as a bad
request rather than an uncaught error.
+ * If there are multiple commands using this step, the last usage
should represent the value returned here.
+ */
+ public Optional<String> getBatchSize() {
+ return null == batchSize ? Optional.empty() :
Optional.of(batchSize);
+ }
+
+ /**
+ * Gets the value of the bulkResults flag supplied using the {@link
GraphTraversal#with(String, Object)} source step.
+ * If there are multiple commands using this step, the last usage
should represent the value returned here.
+ */
+ public Optional<Boolean> getBulkResults() {
+ return null == bulkResults ? Optional.empty() :
Optional.of(bulkResults);
}
@Override
public String toString() {
return "GremlinScriptChecker.Result{" +
"timeout=" + timeout +
- ", requestId='" + requestId + '\'' +
", materializeProperties='" + materializeProperties + '\''
+
+ ", language='" + language + '\'' +
+ ", batchSize=" + batchSize +
+ ", bulkResults=" + bulkResults +
'}';
}
}
diff --git
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptCheckerTest.java
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptCheckerTest.java
index f55660d75e..e597e9c413 100644
---
a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptCheckerTest.java
+++
b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/jsr223/GremlinScriptCheckerTest.java
@@ -34,7 +34,6 @@ public class GremlinScriptCheckerTest {
public void shouldNotFindAResult() {
final GremlinScriptChecker.Result r =
GremlinScriptChecker.parse("g.with(true).V().out('knows')");
assertEquals(Optional.empty(), r.getTimeout());
- assertEquals(Optional.empty(), r.getRequestId());
assertEquals(Optional.empty(), r.getMaterializeProperties());
}
@@ -43,7 +42,6 @@ public class GremlinScriptCheckerTest {
final GremlinScriptChecker.Result r = GremlinScriptChecker.parse("");
assertSame(EMPTY_RESULT, r);
assertEquals(Optional.empty(), r.getTimeout());
- assertEquals(Optional.empty(), r.getRequestId());
assertEquals(Optional.empty(), r.getMaterializeProperties());
}
@@ -64,24 +62,6 @@ public class GremlinScriptCheckerTest {
getTimeout().get().longValue());
}
- @Test
- public void shouldIdentifyRequestIdWithOddSpacing() {
- assertEquals("4F53FB59-CFC9-4984-B477-452073A352FD",
GremlinScriptChecker.parse("g.with('requestId' ,
'4F53FB59-CFC9-4984-B477-452073A352FD').with(true).V().out('knows')").
- getRequestId().get());
- assertEquals("4F53FB59-CFC9-4984-B477-452073A352FD",
GremlinScriptChecker.parse("g.with('requestId' ,
'4F53FB59-CFC9-4984-B477-452073A352FD').with(true).V().out('knows')").
- getRequestId().get());
- assertEquals("4F53FB59-CFC9-4984-B477-452073A352FD",
GremlinScriptChecker.parse("g.with('requestId','4F53FB59-CFC9-4984-B477-452073A352FD').with(true).V().out('knows')").
- getRequestId().get());
- }
-
- @Test
- public void shouldIdentifyRequestIdWithEmbeddedQuote() {
- assertEquals("te\"st",
GremlinScriptChecker.parse("g.with('requestId','te\"st').with(true).V().out('knows')").
- getRequestId().get());
- assertEquals("te\\\"st",
GremlinScriptChecker.parse("g.with('requestId',
\"te\\\"st\").with(true).V().out('knows')").
- getRequestId().get());
- }
-
@Test
public void shouldIdentifyTimeoutWithLowerL() {
assertEquals(1000, GremlinScriptChecker.parse("g.with('timeoutMillis',
1000l).with(true).V().out('knows')").
@@ -106,28 +86,12 @@ public class GremlinScriptCheckerTest {
getTimeout().get().longValue());
}
- @Test
- public void shouldIdentifyRequestIdAsStringKeySingleQuoted() {
- assertEquals("db024fca-ed15-4375-95de-4c6106aef895",
GremlinScriptChecker.parse("g.with('requestId',
'db024fca-ed15-4375-95de-4c6106aef895').with(true).V().out('knows')").
- getRequestId().get());
- assertEquals("db024fca-ed15-4375-95de-4c6106aef895",
GremlinScriptChecker.parse("g.with(\"requestId\",
'db024fca-ed15-4375-95de-4c6106aef895').with(true).V().out('knows')").
- getRequestId().get());
- }
-
@Test
public void shouldIdentifyTimeoutAsStringKeyDoubleQuoted() {
assertEquals(1000,
GremlinScriptChecker.parse("g.with(\"timeoutMillis\",
1000L).with(true).V().out('knows')").
getTimeout().get().longValue());
}
- @Test
- public void shouldIdentifyRequestIdAsStringKeyDoubleQuoted() {
- assertEquals("db024fca-ed15-4375-95de-4c6106aef895",
GremlinScriptChecker.parse("g.with(\"requestId\",
\"db024fca-ed15-4375-95de-4c6106aef895\").with(true).V().out('knows')").
- getRequestId().get());
- assertEquals("db024fca-ed15-4375-95de-4c6106aef895",
GremlinScriptChecker.parse("g.with(\"requestId\",
\"db024fca-ed15-4375-95de-4c6106aef895\").with(true).V().out('knows')").
- getRequestId().get());
- }
-
@Test
public void shouldIdentifyTimeoutAsTokenKey() {
assertEquals(1000,
GremlinScriptChecker.parse("g.with(Tokens.TIMEOUT_MILLIS,
1000L).with(true).V().out('knows')").
@@ -136,14 +100,6 @@ public class GremlinScriptCheckerTest {
getTimeout().get().longValue());
}
- @Test
- public void shouldIdentifyRequestIdAsTokenKey() {
- assertEquals("db024fca-ed15-4375-95de-4c6106aef895",
GremlinScriptChecker.parse("g.with(Tokens.REQUEST_ID,
\"db024fca-ed15-4375-95de-4c6106aef895\").with(true).V().out('knows')").
- getRequestId().get());
- assertEquals("db024fca-ed15-4375-95de-4c6106aef895",
GremlinScriptChecker.parse("g.with(Tokens.REQUEST_ID,
\"db024fca-ed15-4375-95de-4c6106aef895\").with(true).V().out('knows')").
- getRequestId().get());
- }
-
@Test
public void shouldIdentifyTimeoutAsTokenKeyWithoutClassName() {
assertEquals(1000, GremlinScriptChecker.parse("g.with(TIMEOUT_MILLIS,
1000L).with(true).V().out('knows')").
@@ -152,14 +108,6 @@ public class GremlinScriptCheckerTest {
getTimeout().get().longValue());
}
- @Test
- public void shouldIdentifyRequestIdAsTokenKeyWithoutClassName() {
- assertEquals("db024fca-ed15-4375-95de-4c6106aef895",
GremlinScriptChecker.parse("g.with(REQUEST_ID,
\"db024fca-ed15-4375-95de-4c6106aef895\").with(true).V().out('knows')").
- getRequestId().get());
- assertEquals("db024fca-ed15-4375-95de-4c6106aef895",
GremlinScriptChecker.parse("g.with(REQUEST_ID,
'db024fca-ed15-4375-95de-4c6106aef895').with(true).V().out('knows')").
- getRequestId().get());
- }
-
@Test
public void shouldIdentifyMultipleTimeouts() {
assertEquals(6000, GremlinScriptChecker.parse("g.with('timeoutMillis',
1000L).with(true).V().out('knows');" +
@@ -176,22 +124,6 @@ public class GremlinScriptCheckerTest {
getTimeout().get().longValue());
}
- @Test
- public void shouldIdentifyMultipleRequestIds() {
- assertEquals("test9", GremlinScriptChecker.parse("g.with('requestId',
'test1').with(true).V().out('knows');" +
- "g.with('requestId',
'test2').with(true).V().out('knows');\n" +
- "
//g.with('requestId', 'test3').with(true).V().out('knows');\n" +
- " /*
g.with('requestId', 'test4').with(true).V().out('knows');*/\n" +
- " /*
\n" +
- "g.with('requestId',
'test5').with(true).V().out('knows'); \n" +
- "*/ \n" +
- "
g.with('requestId', 'test6').with(true).V().out('knows');\n" +
- "
g.with(Tokens.REQUEST_ID, 'test7').with(true).V().out('knows');\n" +
- "
g.with(REQUEST_ID, 'test8').with(true).V().out('knows');\n" +
- "
g.with('requestId', 'test9').with(true).V().out('knows');").
- getRequestId().get());
- }
-
@Test
public void shouldIdentifyMaterializePropertiesSingleQuoted() {
assertEquals("all",
GremlinScriptChecker.parse("g.with('materializeProperties',
'all').with(true).V().out('knows')").
@@ -239,9 +171,8 @@ public class GremlinScriptCheckerTest {
@Test
public void shouldFindAllResults() {
final GremlinScriptChecker.Result r = GremlinScriptChecker.parse(
- "g.with('timeoutMillis', 1000).with(true).with(REQUEST_ID,
\"db024fca-ed15-4375-95de-4c6106aef895\").with(\"materializeProperties\",
'all').V().out('knows')");
+ "g.with('timeoutMillis',
1000).with(true).with(\"materializeProperties\", 'all').V().out('knows')");
assertEquals(1000, r.getTimeout().get().longValue());
- assertEquals("db024fca-ed15-4375-95de-4c6106aef895",
r.getRequestId().get());
assertEquals("all", r.getMaterializeProperties().get());
}
@@ -449,4 +380,141 @@ public class GremlinScriptCheckerTest {
" addE('bridge').from('o').to('r').iterate()").
getTimeout().get().longValue());
}
+
+ @Test
+ public void shouldIdentifyLanguageStringKey() {
+ assertEquals("gremlin-groovy",
GremlinScriptChecker.parse("g.with('language',
'gremlin-groovy').V().out('knows')").
+ getLanguage().get());
+ assertEquals("gremlin-groovy",
GremlinScriptChecker.parse("g.with(\"language\",
\"gremlin-groovy\").V().out('knows')").
+ getLanguage().get());
+ }
+
+ @Test
+ public void shouldIdentifyLanguageAsTokenKey() {
+ assertEquals("gremlin-groovy",
GremlinScriptChecker.parse("g.with(ARGS_LANGUAGE,
'gremlin-groovy').V().out('knows')").
+ getLanguage().get());
+ assertEquals("gremlin-groovy",
GremlinScriptChecker.parse("g.with(Tokens.ARGS_LANGUAGE,
'gremlin-groovy').V().out('knows')").
+ getLanguage().get());
+ }
+
+ @Test
+ public void shouldIdentifyMultipleLanguagesUsingLast() {
+ assertEquals("gremlin-lang",
GremlinScriptChecker.parse("g.with('language',
'gremlin-groovy').with('language', 'gremlin-lang').V()").
+ getLanguage().get());
+ }
+
+ @Test
+ public void shouldIdentifyBatchSizeStringKey() {
+ assertEquals("10", GremlinScriptChecker.parse("g.with('batchSize',
10).V().out('knows')").
+ getBatchSize().get());
+ assertEquals("10", GremlinScriptChecker.parse("g.with(\"batchSize\",
10).V().out('knows')").
+ getBatchSize().get());
+ }
+
+ @Test
+ public void shouldIdentifyBatchSizeAsTokenKey() {
+ assertEquals("25", GremlinScriptChecker.parse("g.with(ARGS_BATCH_SIZE,
25).V().out('knows')").
+ getBatchSize().get());
+ assertEquals("25",
GremlinScriptChecker.parse("g.with(Tokens.ARGS_BATCH_SIZE,
25).V().out('knows')").
+ getBatchSize().get());
+ }
+
+ @Test
+ public void shouldIdentifyMultipleBatchSizesUsingLast() {
+ assertEquals("64", GremlinScriptChecker.parse("g.with('batchSize',
10).with('batchSize', 64).V()").
+ getBatchSize().get());
+ }
+
+ @Test
+ public void shouldCaptureBatchSizeAboveIntegerMaxUnparsed() {
+ // the checker captures the raw string without parsing, so a value
above Integer.MAX_VALUE is returned as-is
+ // (rather than throwing) and the server can reject it as a bad
request.
+ assertEquals("2147483648",
GremlinScriptChecker.parse("g.with('batchSize', 2147483648).V()").
+ getBatchSize().get());
+ }
+
+ @Test
+ public void shouldCaptureNegativeBatchSizeUnparsed() {
+ assertEquals("-1", GremlinScriptChecker.parse("g.with('batchSize',
-1).V()").
+ getBatchSize().get());
+ }
+
+ @Test
+ public void shouldIdentifyBulkResultsStringKey() {
+ assertEquals(Boolean.TRUE,
GremlinScriptChecker.parse("g.with('bulkResults', true).V().out('knows')").
+ getBulkResults().get());
+ assertEquals(Boolean.FALSE,
GremlinScriptChecker.parse("g.with(\"bulkResults\", false).V().out('knows')").
+ getBulkResults().get());
+ }
+
+ @Test
+ public void shouldIdentifyBulkResultsCaseInsensitive() {
+ assertEquals(Boolean.TRUE,
GremlinScriptChecker.parse("g.with('bulkResults', TRUE).V().out('knows')").
+ getBulkResults().get());
+ assertEquals(Boolean.FALSE,
GremlinScriptChecker.parse("g.with('bulkResults', False).V().out('knows')").
+ getBulkResults().get());
+ }
+
+ @Test
+ public void shouldIdentifyBulkResultsAsTokenKey() {
+ assertEquals(Boolean.TRUE,
GremlinScriptChecker.parse("g.with(BULK_RESULTS, true).V().out('knows')").
+ getBulkResults().get());
+ assertEquals(Boolean.TRUE,
GremlinScriptChecker.parse("g.with(Tokens.BULK_RESULTS,
true).V().out('knows')").
+ getBulkResults().get());
+ }
+
+ @Test
+ public void shouldIdentifyMultipleBulkResultsUsingLast() {
+ assertEquals(Boolean.FALSE,
GremlinScriptChecker.parse("g.with('bulkResults', true).with('bulkResults',
false).V()").
+ getBulkResults().get());
+ }
+
+ @Test
+ public void shouldNotIdentifyInvalidBulkResults() {
+ assertEquals(Optional.empty(),
GremlinScriptChecker.parse("g.with('bulkResults', maybe).V()").
+ getBulkResults());
+ }
+
+ @Test
+ public void shouldNotIdentifyBindingsFromScript() {
+ // parameters is intentionally NOT scraped from a script
(field/header-only)
+ final GremlinScriptChecker.Result r =
GremlinScriptChecker.parse("g.with('parameters', '[\"x\":1]').V(x)");
+ assertEquals(Optional.empty(), r.getTimeout());
+ assertEquals(Optional.empty(), r.getLanguage());
+ assertEquals(Optional.empty(), r.getBatchSize());
+ assertEquals(Optional.empty(), r.getBulkResults());
+ }
+
+ @Test
+ public void shouldNotIdentifyTransactionIdFromScript() {
+ // transactionId is intentionally NOT scraped from a script
(field/header-only)
+ final GremlinScriptChecker.Result r =
GremlinScriptChecker.parse("g.with('transactionId', 'abc').V()");
+ assertEquals(Optional.empty(), r.getTimeout());
+ assertEquals(Optional.empty(), r.getLanguage());
+ assertEquals(Optional.empty(), r.getBatchSize());
+ assertEquals(Optional.empty(), r.getBulkResults());
+ }
+
+ @Test
+ public void shouldNotIdentifyGFromScript() {
+ // g is intentionally NOT scraped from a script due to the complex
interactions between different script
+ // engines and transactions - it must be supplied as a request field.
+ final GremlinScriptChecker.Result r =
GremlinScriptChecker.parse("g.with('g', 'myGraph').V().out('knows')");
+ assertEquals(Optional.empty(), r.getTimeout());
+ assertEquals(Optional.empty(), r.getLanguage());
+ assertEquals(Optional.empty(), r.getBatchSize());
+ assertEquals(Optional.empty(), r.getBulkResults());
+ }
+
+ @Test
+ public void shouldFindAllFiveResults() {
+ final GremlinScriptChecker.Result r = GremlinScriptChecker.parse(
+ "g.with('timeoutMillis', 1000).with('materializeProperties',
'all').with('language', 'gremlin-groovy')." +
+ "with('batchSize', 32).with('bulkResults',
true).V().out('knows')");
+ assertEquals(1000, r.getTimeout().get().longValue());
+ assertEquals("all", r.getMaterializeProperties().get());
+ assertEquals("gremlin-groovy", r.getLanguage().get());
+ assertEquals("32", r.getBatchSize().get());
+ assertEquals(Boolean.TRUE, r.getBulkResults().get());
+ }
}
diff --git
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java
index b80c5fc9e5..b080653439 100644
---
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java
+++
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/Context.java
@@ -20,9 +20,12 @@ package org.apache.tinkerpop.gremlin.server;
import org.apache.tinkerpop.gremlin.process.traversal.Path;
import io.netty.channel.ChannelHandlerContext;
+import io.netty.handler.codec.http.HttpHeaders;
import org.apache.tinkerpop.gremlin.groovy.engine.GremlinExecutor;
import org.apache.tinkerpop.gremlin.jsr223.GremlinScriptChecker;
import
org.apache.tinkerpop.gremlin.process.traversal.traverser.util.AbstractTraverser;
+import org.apache.tinkerpop.gremlin.server.handler.StateKey;
+import org.apache.tinkerpop.gremlin.server.util.GremlinError;
import org.apache.tinkerpop.gremlin.structure.Element;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.util.reference.ReferenceFactory;
@@ -50,7 +53,9 @@ public class Context {
private final ScheduledExecutorService scheduledExecutorService;
private final long requestTimeout;
private final String materializeProperties;
- private final Object gremlinArgument;
+ private final String language;
+ private final String batchSize;
+ private final boolean bulkResults;
private final RequestType requestType;
private ScheduledFuture<?> timeoutExecutor = null;
private boolean timeoutExecutorGrabbed = false;
@@ -74,10 +79,18 @@ public class Context {
// order of calls matter as one depends on the next
final String gremlin = requestMessage.getGremlin();
- this.gremlinArgument = gremlin;
this.requestType = RequestType.fromGremlin(gremlin);
- this.requestTimeout = determineTimeout();
- this.materializeProperties = determineMaterializeProperties();
+
+ // Parse the script once and share the result across all per-request
option resolution below. Every
+ // determine*() method applies the same precedence for its option:
+ // script-embedded with() > explicit request field > request header
(bulkResults only) > server/connection default
+ // The individual methods are not re-documented with this rule; they
only note behavior unique to that option.
+ final GremlinScriptChecker.Result scriptOptions =
GremlinScriptChecker.parse(gremlin);
+ this.requestTimeout = determineTimeout(scriptOptions);
+ this.materializeProperties =
determineMaterializeProperties(scriptOptions);
+ this.language = determineLanguage(scriptOptions);
+ this.batchSize = determineBatchSize(scriptOptions);
+ this.bulkResults = determineBulkResults(scriptOptions);
this.transactionId =
requestMessage.getField(Tokens.ARGS_TRANSACTION_ID);
}
@@ -101,10 +114,7 @@ public class Context {
}
/**
- * The timeout for the request. If the request is a script it examines the
script for a timeout setting using
- * {@code with()}. If that is not found then it examines the request
itself to see if the timeout is provided by
- * {@link Tokens#TIMEOUT_MILLIS}. If that is not provided then the {@link
Settings#timeoutMillis} is
- * utilized as the default.
+ * The timeout in milliseconds for the request. Resolved in the
constructor (see the precedence note there).
*/
public long getRequestTimeout() {
return requestTimeout;
@@ -114,6 +124,40 @@ public class Context {
return materializeProperties;
}
+ /**
+ * The language resolved for the request (see the precedence note in the
constructor).
+ */
+ public String getLanguage() {
+ return language;
+ }
+
+ /**
+ * The batch size resolved for the request, parsed to a positive {@code
int}. Parsing happens here (rather than in
+ * the {@link Context} constructor) so that an invalid value -
non-numeric, non-positive, or greater than
+ * {@link Integer#MAX_VALUE} - is surfaced as a bad request via {@link
ProcessingException} rather than an uncaught
+ * error. Callers must be on a path that turns a {@link
ProcessingException} into a response (i.e. before the
+ * {@code 200 OK} is committed).
+ */
+ public int getBatchSize() throws ProcessingException {
+ final int parsed;
+ try {
+ parsed = Integer.parseInt(batchSize);
+ } catch (NumberFormatException nfe) {
+ throw new ProcessingException(GremlinError.batchSize(batchSize));
+ }
+ if (parsed <= 0) {
+ throw new ProcessingException(GremlinError.batchSize(batchSize));
+ }
+ return parsed;
+ }
+
+ /**
+ * Whether results should be bulked for the request (see the precedence
note in the constructor).
+ */
+ public boolean getBulkResults() {
+ return bulkResults;
+ }
+
public ScheduledExecutorService getScheduledExecutorService() {
return scheduledExecutorService;
}
@@ -207,20 +251,14 @@ public class Context {
return gremlinExecutor;
}
- private long determineTimeout() {
- // per-request timeout override falls back to the server-configured
default when not supplied
+ private long determineTimeout(final GremlinScriptChecker.Result
scriptOptions) {
final Long timeoutMillis =
requestMessage.getField(Tokens.TIMEOUT_MILLIS);
final long seto = (null != timeoutMillis) ? timeoutMillis :
settings.getTimeoutMillis();
-
- // override the timeout if the lifecycle has a value assigned. if the
script contains with(timeout)
- // options then allow that value to override what's provided on the
lifecycle
- final Optional<Long> timeoutDefinedInScript =
GremlinScriptChecker.parse(gremlinArgument.toString()).getTimeout();
-
- return timeoutDefinedInScript.orElse(seto);
+ return scriptOptions.getTimeout().orElse(seto);
}
- private String determineMaterializeProperties() {
- final Optional<String> mp =
GremlinScriptChecker.parse(gremlinArgument.toString()).getMaterializeProperties();
+ private String determineMaterializeProperties(final
GremlinScriptChecker.Result scriptOptions) {
+ final Optional<String> mp = scriptOptions.getMaterializeProperties();
if (mp.isPresent())
return mp.get().equals(Tokens.MATERIALIZE_PROPERTIES_TOKENS)
? Tokens.MATERIALIZE_PROPERTIES_TOKENS
@@ -233,6 +271,43 @@ public class Context {
: Tokens.MATERIALIZE_PROPERTIES_ALL;
}
+ private String determineLanguage(final GremlinScriptChecker.Result
scriptOptions) {
+ final Optional<String> lang = scriptOptions.getLanguage();
+ if (lang.isPresent()) return lang.get();
+
+ final String language = requestMessage.getField(Tokens.ARGS_LANGUAGE);
+ return (null != language) ? language : "gremlin-lang";
+ }
+
+ private String determineBatchSize(final GremlinScriptChecker.Result
scriptOptions) {
+ // the value is kept as a raw string here and only parsed/validated in
getBatchSize(), so an invalid value does
+ // not throw while constructing Context (which runs outside the
request's error-handling path).
+ final Optional<String> bs = scriptOptions.getBatchSize();
+ if (bs.isPresent()) return bs.get();
+
+ final Integer batchSize =
requestMessage.getField(Tokens.ARGS_BATCH_SIZE);
+ return String.valueOf((null != batchSize) ? batchSize :
settings.resultIterationBatchSize);
+ }
+
+ private boolean determineBulkResults(final GremlinScriptChecker.Result
scriptOptions) {
+ final Optional<Boolean> br = scriptOptions.getBulkResults();
+ if (br.isPresent()) return br.get();
+
+ final Object bulkResultsField =
requestMessage.getField(Tokens.BULK_RESULTS);
+ if (null != bulkResultsField) return
Boolean.parseBoolean(bulkResultsField.toString());
+
+ // the request header is a fallback below the request field. channel
may be absent in unit tests.
+ if (null != channelHandlerContext && null !=
channelHandlerContext.channel()) {
+ final HttpHeaders headers =
channelHandlerContext.channel().attr(StateKey.REQUEST_HEADERS).get();
+ if (null != headers) {
+ final String bulkResultsHeader =
headers.get(Tokens.BULK_RESULTS);
+ if (null != bulkResultsHeader) return
Boolean.parseBoolean(bulkResultsHeader);
+ }
+ }
+
+ return false;
+ }
+
public void handleDetachment(final List<Object> aggregate) {
if (!aggregate.isEmpty() &&
!this.getMaterializeProperties().equals(Tokens.MATERIALIZE_PROPERTIES_ALL)) {
for (int i = 0; i < aggregate.size(); i++) {
diff --git
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
index 13531f856d..01df7e9297 100644
---
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
+++
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpGremlinEndpointHandler.java
@@ -81,7 +81,6 @@ import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
-import java.util.Objects;
import java.util.Optional;
import java.util.Set;
import java.util.UUID;
@@ -192,9 +191,9 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
final Optional<UnmanagedTransaction> txForRequest =
isTransactionalOp ?
transactionManager.get(requestCtx.getTransactionId()) : Optional.empty();
- // per-request timeout override falls back to the server-configured
default when not supplied
- final Long timeoutMillis =
requestMessage.getField(Tokens.TIMEOUT_MILLIS);
- final long seto = (null != timeoutMillis) ? timeoutMillis :
requestCtx.getSettings().getTimeoutMillis();
+ // consume the Context-resolved timeout (rather than re-reading the
field) so that a timeout embedded in a
+ // raw-string script's with() is actually applied.
+ final long seto = requestCtx.getRequestTimeout();
final FutureTask<Void> evalFuture = new FutureTask<>(() -> {
try {
@@ -214,8 +213,14 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
}
// Validate the request before any transaction lifecycle side
effects.
- final Map<String, Object> args = requestMessage.getFields();
- final String language = args.containsKey(Tokens.ARGS_LANGUAGE)
? (String) args.get(Tokens.ARGS_LANGUAGE) : "gremlin-lang";
+
+ // batchSize must parse to a positive int
(1..Integer.MAX_VALUE); getBatchSize() throws a
+ // ProcessingException (-> bad request) for a non-numeric,
non-positive, or out-of-range value. This
+ // runs before any transaction lifecycle side effects and
before the 200 OK is committed.
+ requestCtx.getBatchSize();
+
+ // validate script engine availability.
+ final String language = requestCtx.getLanguage();
if
(gremlinExecutor.getScriptEngineManager().getEngineByName(language) == null) {
throw new
ProcessingException(GremlinError.scriptEngineNotAvailable(language));
}
@@ -434,8 +439,10 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
private void iterateScriptEvalResult(final Context context,
MessageSerializer<?> serializer, final RequestMessage message,
final HttpResponseCoordinator
coordinator)
throws ProcessingException, InterruptedException, ScriptException {
- final Map<String, Object> args = message.getFields();
- final String language = args.containsKey(Tokens.ARGS_LANGUAGE) ?
(String) args.get(Tokens.ARGS_LANGUAGE) : "gremlin-lang";
+ final String language = context.getLanguage();
+ // resolve batchSize here (this method declares ProcessingException)
so it is not parsed again downstream in
+ // handleIterator; it was already validated by the pre-200 guard, so
this will not throw.
+ final int resultIterationBatchSize = context.getBatchSize();
final GremlinScriptEngine scriptEngine =
gremlinExecutor.getScriptEngineManager().getEngineByName(language);
if (scriptEngine == null) {
@@ -463,23 +470,19 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
try {
final Object result = scriptEngine.eval(message.getGremlin(),
mergedBindings);
- final String bulkingSetting =
context.getChannelHandlerContext().channel().attr(StateKey.REQUEST_HEADERS).get().get(Tokens.BULK_RESULTS);
- // bulking only applies if it's gremlin-lang, and per request
token setting takes precedence over header setting.
+ // bulking only applies if it's gremlin-lang and the serializer
supports it.
// The serializer check is temporarily needed because GraphSON
hasn't been removed yet and doesn't support bulking.
- final boolean bulking = language.equals("gremlin-lang") &&
serializer instanceof GraphBinaryMessageSerializerV4 ?
- (args.containsKey(Tokens.BULK_RESULTS) ?
- Objects.equals(args.get(Tokens.BULK_RESULTS),
"true") :
- Objects.equals(bulkingSetting, "true")) :
- false;
+ final boolean bulking = language.equals("gremlin-lang") &&
serializer instanceof GraphBinaryMessageSerializerV4 &&
+ context.getBulkResults();
if (bulking) {
// optimization for driver requests
((Traversal.Admin<?, ?>) result).applyStrategies();
itty = new TraverserIterator((Traversal.Admin<?, ?>) result);
- handleIterator(context, itty, coordinator, true);
+ handleIterator(context, itty, coordinator, true,
resultIterationBatchSize);
} else {
itty = IteratorUtils.asIterator(result);
- handleIterator(context, itty, coordinator, false);
+ handleIterator(context, itty, coordinator, false,
resultIterationBatchSize);
}
if (autoCommit && graph.tx().isOpen()) graph.tx().commit();
@@ -598,10 +601,8 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
return bindings;
}
- private void handleIterator(final Context context, final Iterator itty,
final HttpResponseCoordinator coordinator, final boolean bulking) throws
InterruptedException {
+ private void handleIterator(final Context context, final Iterator itty,
final HttpResponseCoordinator coordinator, final boolean bulking, final int
resultIterationBatchSize) throws InterruptedException {
final ChannelHandlerContext nettyContext =
context.getChannelHandlerContext();
- final RequestMessage msg = context.getRequestMessage();
- final Settings settings = context.getSettings();
// used to limit warnings for when netty fills the buffer and hits the
high watermark - prevents
// over-logging of the same message.
@@ -619,9 +620,7 @@ public class HttpGremlinEndpointHandler extends
SimpleChannelInboundHandler<Requ
return;
}
- // the batch size can be overridden by the request
- final int resultIterationBatchSize = (Integer)
msg.optionalField(Tokens.ARGS_BATCH_SIZE)
- .orElse(settings.resultIterationBatchSize);
+ // batch size was resolved and validated by the caller.
List<Object> aggregate = new ArrayList<>(resultIterationBatchSize);
// use an external control to manage the loop as opposed to just
checking hasNext() in the while. this
diff --git
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinError.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinError.java
index 4e9bc9ffe7..414e007292 100644
---
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinError.java
+++
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/GremlinError.java
@@ -88,6 +88,12 @@ public class GremlinError {
return new GremlinError(HttpResponseStatus.BAD_REQUEST, message,
"InvalidRequestException");
}
+ public static GremlinError batchSize(final String batchSize) {
+ final String message = String.format("The message specifies a
batchSize of %s but it must be an integer between 1 and %s",
+ batchSize, Integer.MAX_VALUE);
+ return new GremlinError(HttpResponseStatus.BAD_REQUEST, message,
"InvalidRequestException");
+ }
+
public static GremlinError binding(final String aliased) {
final String message = String.format("Could not alias [%s] to [%s] as
[%s] not in the Graph or TraversalSource global bindings",
Tokens.ARGS_G, aliased, aliased);
diff --git
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ContextTest.java
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ContextTest.java
index 6de24da96c..1087e59e79 100644
---
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ContextTest.java
+++
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ContextTest.java
@@ -20,6 +20,7 @@ package org.apache.tinkerpop.gremlin.server;
import io.netty.channel.ChannelHandlerContext;
import nl.altindag.log.LogCaptor;
+import org.apache.tinkerpop.gremlin.util.Tokens;
import org.apache.tinkerpop.gremlin.util.message.RequestMessage;
import org.junit.AfterClass;
import org.junit.Before;
@@ -28,6 +29,9 @@ import org.junit.Test;
import org.mockito.Mockito;
import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertThrows;
+import static org.junit.Assert.assertTrue;
public class ContextTest {
@@ -76,4 +80,143 @@ public class ContextTest {
// "all" is default value
assertEquals("all", context.getMaterializeProperties());
}
-}
\ No newline at end of file
+
+ private static Context newContext(final RequestMessage request) {
+ final ChannelHandlerContext ctx =
Mockito.mock(ChannelHandlerContext.class);
+ return new Context(request, ctx, new Settings(), null, null, null);
+ }
+
+ @Test
+ public void shouldResolveTimeoutFromScriptOverField() {
+ final RequestMessage request =
RequestMessage.build("g.with('timeoutMillis', 1000).V()")
+ .addTimeoutMillis(5000).create();
+ assertEquals(1000, newContext(request).getRequestTimeout());
+ }
+
+ @Test
+ public void shouldResolveTimeoutFromFieldWhenNoScript() {
+ final RequestMessage request =
RequestMessage.build("g.V()").addTimeoutMillis(5000).create();
+ assertEquals(5000, newContext(request).getRequestTimeout());
+ }
+
+ @Test
+ public void shouldResolveTimeoutFromDefaultWhenNeither() {
+ final RequestMessage request = RequestMessage.build("g.V()").create();
+ assertEquals(new Settings().getTimeoutMillis(),
newContext(request).getRequestTimeout());
+ }
+
+ @Test
+ public void shouldResolveMaterializePropertiesFromScriptOverField() {
+ final RequestMessage request =
RequestMessage.build("g.with('materializeProperties', 'tokens').V()")
+ .addMaterializeProperties("all").create();
+ assertEquals("tokens", newContext(request).getMaterializeProperties());
+ }
+
+ @Test
+ public void shouldResolveMaterializePropertiesFromFieldWhenNoScript() {
+ final RequestMessage request =
RequestMessage.build("g.V()").addMaterializeProperties("tokens").create();
+ assertEquals("tokens", newContext(request).getMaterializeProperties());
+ }
+
+ @Test
+ public void shouldResolveMaterializePropertiesFromDefaultWhenNeither() {
+ final RequestMessage request = RequestMessage.build("g.V()").create();
+ assertEquals("all", newContext(request).getMaterializeProperties());
+ }
+
+ @Test
+ public void shouldResolveLanguageFromScriptOverField() {
+ final RequestMessage request =
RequestMessage.build("g.with('language', 'gremlin-groovy').V()")
+ .addLanguage("gremlin-lang").create();
+ assertEquals("gremlin-groovy", newContext(request).getLanguage());
+ }
+
+ @Test
+ public void shouldResolveLanguageFromFieldWhenNoScript() {
+ final RequestMessage request =
RequestMessage.build("g.V()").addLanguage("gremlin-groovy").create();
+ assertEquals("gremlin-groovy", newContext(request).getLanguage());
+ }
+
+ @Test
+ public void shouldResolveLanguageFromDefaultWhenNeither() {
+ final RequestMessage request = RequestMessage.build("g.V()").create();
+ assertEquals("gremlin-lang", newContext(request).getLanguage());
+ }
+
+ @Test
+ public void shouldResolveBatchSizeFromScriptOverField() throws Exception {
+ final RequestMessage request =
RequestMessage.build("g.with('batchSize', 10).V()")
+ .addChunkSize(500).create();
+ assertEquals(10, newContext(request).getBatchSize());
+ }
+
+ @Test
+ public void shouldResolveBatchSizeFromFieldWhenNoScript() throws Exception
{
+ final RequestMessage request =
RequestMessage.build("g.V()").addChunkSize(500).create();
+ assertEquals(500, newContext(request).getBatchSize());
+ }
+
+ @Test
+ public void shouldResolveBatchSizeFromDefaultWhenNeither() throws
Exception {
+ final RequestMessage request = RequestMessage.build("g.V()").create();
+ assertEquals(new Settings().resultIterationBatchSize,
newContext(request).getBatchSize());
+ }
+
+ @Test
+ public void shouldThrowOnNonPositiveBatchSize() {
+ // getBatchSize() parses and validates, throwing a bad-request
ProcessingException for a non-positive value
+ // (from either the script or the request field) rather than returning
a value that would stall iteration.
+ final RequestMessage fromScript =
RequestMessage.build("g.with('batchSize', 0).V()").create();
+ assertThrows(ProcessingException.class, () ->
newContext(fromScript).getBatchSize());
+
+ final RequestMessage negativeFromScript =
RequestMessage.build("g.with('batchSize', -1).V()").create();
+ assertThrows(ProcessingException.class, () ->
newContext(negativeFromScript).getBatchSize());
+
+ final RequestMessage fromField =
RequestMessage.build("g.V()").addChunkSize(0).create();
+ assertThrows(ProcessingException.class, () ->
newContext(fromField).getBatchSize());
+ }
+
+ @Test
+ public void shouldThrowOnBatchSizeAboveIntegerMax() {
+ // a script-embedded value above Integer.MAX_VALUE cannot be applied
and is rejected as a bad request rather
+ // than throwing an uncaught NumberFormatException while constructing
Context.
+ final RequestMessage request =
RequestMessage.build("g.with('batchSize', 2147483648).V()").create();
+ assertThrows(ProcessingException.class, () ->
newContext(request).getBatchSize());
+ }
+
+ @Test
+ public void shouldResolveBulkResultsFromScriptOverField() {
+ final RequestMessage request =
RequestMessage.build("g.with('bulkResults', false).V()")
+ .addBulkResults(true).create();
+ assertFalse(newContext(request).getBulkResults());
+ }
+
+ @Test
+ public void shouldResolveBulkResultsFromFieldWhenNoScript() {
+ final RequestMessage request =
RequestMessage.build("g.V()").addBulkResults(true).create();
+ assertTrue(newContext(request).getBulkResults());
+ }
+
+ @Test
+ public void shouldResolveBulkResultsFromDefaultWhenNeither() {
+ final RequestMessage request = RequestMessage.build("g.V()").create();
+ assertFalse(newContext(request).getBulkResults());
+ }
+
+ @Test
+ public void shouldNotHonorParametersFromScript() {
+ // an embedded with('parameters', ...) must not become the request's
parameters; only the field is authoritative.
+ final RequestMessage request =
RequestMessage.build("g.with('parameters', '[\"x\":1]').V(x)")
+ .addParameters("[\"y\":2]").create();
+ final Context context = newContext(request);
+ // Context does not expose parsed parameters from the script; the
field value remains the source of truth.
+ assertEquals("[\"y\":2]",
context.getRequestMessage().getField(Tokens.ARGS_PARAMETERS));
+ }
+
+ @Test
+ public void shouldNotHonorTransactionIdFromScript() {
+ final RequestMessage request =
RequestMessage.build("g.with('transactionId', 'fromScript').V()")
+ .addTransactionId("fromField").create();
+ assertEquals("fromField", newContext(request).getTransactionId());
+ }
+}
diff --git
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java
index 4bf10dc346..77cdfa8999 100644
---
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java
+++
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerHttpIntegrateTest.java
@@ -113,6 +113,7 @@ public class GremlinServerHttpIntegrateTest extends
AbstractGremlinServerIntegra
case "should200OnPOSTTransactionalGraph":
case "shouldRollbackOnFailedMutatingTraversal":
case "shouldCommitMutatingTraversalWithEmptyResult":
+ case "should400OnTransactionBeginWithNonPositiveBatchSize":
useTinkerTransactionGraph(settings);
break;
case "should200OnPOSTTransactionalGraphInStrictMode":
@@ -1364,6 +1365,144 @@ public class GremlinServerHttpIntegrateTest extends
AbstractGremlinServerIntegra
}
}
+ @Test(timeout = 10000) // Add test timeout to prevent incorrect timeout
behavior from stopping test run.
+ public void shouldAcceptTimeoutMillisEmbeddedInScript() throws Exception {
+ // Regression: a with('timeoutMillis', ...) embedded in a raw-string
script must be honored by the server.
+ // Previously the server computed but never applied this value, so the
query ran to completion instead of
+ // timing out.
+ final String body = "{ \"gremlin\": \"" + "g.with('timeoutMillis',
100).inject(1).sideEffect{Thread.sleep(50000)}"
+ + "\",\"language\":\"gremlin-groovy\"}";
+ final CloseableHttpClient httpclient = HttpClients.createDefault();
+ final HttpPost httppost = new
HttpPost(TestClientFactory.createURLString());
+ httppost.addHeader("Content-Type", "application/json");
+ httppost.setEntity(new StringEntity(body, Consts.UTF_8));
+
+ try (final CloseableHttpResponse response =
httpclient.execute(httppost)) {
+ assertEquals(200, response.getStatusLine().getStatusCode());
+ final String json = EntityUtils.toString(response.getEntity());
+ assertTrue(json.contains("timeout occurred"));
+ }
+ }
+
+ @Test
+ public void shouldAcceptMaterializePropertiesTokensEmbeddedInScript()
throws Exception {
+ final String body = "{ \"gremlin\": \"" +
"gmodern.with('materializeProperties', 'tokens').V().limit(1)"
+ + "\",\"language\":\"gremlin-groovy\"}";
+ final CloseableHttpClient httpclient = HttpClients.createDefault();
+ final HttpPost httppost = new
HttpPost(TestClientFactory.createURLString());
+ httppost.addHeader("Content-Type", "application/json");
+ httppost.setEntity(new StringEntity(body, Consts.UTF_8));
+
+ try (final CloseableHttpResponse response =
httpclient.execute(httppost)) {
+ assertEquals(200, response.getStatusLine().getStatusCode());
+ final String json = EntityUtils.toString(response.getEntity());
+ final JsonNode node = mapper.readTree(json);
+
assertNull(node.get("result").get(TOKEN_DATA).get(GraphSONTokens.VALUEPROP).get(0).get(GraphSONTokens.VALUEPROP).get(GraphSONTokens.PROPERTIES));
+ }
+ }
+
+ @Test
+ public void shouldPreferScriptEmbeddedTimeoutOverRequestField() throws
Exception {
+ // Precedence: script-embedded with() wins over the request field. The
script sets a small timeout that fires;
+ // the (larger) field value must not override it.
+ final String body = "{ \"gremlin\": \"" + "g.with('timeoutMillis',
100).inject(1).sideEffect{Thread.sleep(5000)}"
+ + "\",\"language\":\"gremlin-groovy\",\"" + TIMEOUT_MILLIS +
"\":\"8000\"}";
+ final CloseableHttpClient httpclient = HttpClients.createDefault();
+ final HttpPost httppost = new
HttpPost(TestClientFactory.createURLString());
+ httppost.addHeader("Content-Type", "application/json");
+ httppost.setEntity(new StringEntity(body, Consts.UTF_8));
+
+ try (final CloseableHttpResponse response =
httpclient.execute(httppost)) {
+ assertEquals(200, response.getStatusLine().getStatusCode());
+ final String json = EntityUtils.toString(response.getEntity());
+ assertTrue(json.contains("timeout occurred"));
+ }
+ }
+
+ @Test
+ public void should400OnScriptEmbeddedNonPositiveBatchSize() throws
Exception {
+ // a non-positive batchSize would stall result iteration, so it is
rejected as a bad request.
+ try (final CloseableHttpClient httpclient =
HttpClients.createDefault()) {
+ for (final String batchSize : new String[] {"0", "-1"}) {
+ final String body = "{ \"gremlin\": \"" + "g.with('batchSize',
" + batchSize + ").inject(1)" + "\",\"language\":\"gremlin-groovy\"}";
+ final HttpPost httppost = new
HttpPost(TestClientFactory.createURLString());
+ httppost.addHeader("Content-Type", "application/json");
+ httppost.setEntity(new StringEntity(body, Consts.UTF_8));
+
+ try (final CloseableHttpResponse response =
httpclient.execute(httppost)) {
+ assertEquals(400,
response.getStatusLine().getStatusCode());
+ }
+ }
+ }
+ }
+
+ @Test
+ public void should400OnScriptEmbeddedOverflowBatchSize() throws Exception {
+ // a batchSize above Integer.MAX_VALUE cannot be applied; it must be a
clean 400, not an uncaught error (500).
+ final String body = "{ \"gremlin\": \"" + "g.with('batchSize',
2147483648).inject(1)" + "\",\"language\":\"gremlin-groovy\"}";
+ final CloseableHttpClient httpclient = HttpClients.createDefault();
+ final HttpPost httppost = new
HttpPost(TestClientFactory.createURLString());
+ httppost.addHeader("Content-Type", "application/json");
+ httppost.setEntity(new StringEntity(body, Consts.UTF_8));
+
+ try (final CloseableHttpResponse response =
httpclient.execute(httppost)) {
+ assertEquals(400, response.getStatusLine().getStatusCode());
+ }
+ }
+
+ @Test
+ public void should400OnRequestFieldNonPositiveBatchSize() throws Exception
{
+ final String body = "{ \"gremlin\": \"g.inject(1)\",\"batchSize\":0}";
+ final CloseableHttpClient httpclient = HttpClients.createDefault();
+ final HttpPost httppost = new
HttpPost(TestClientFactory.createURLString());
+ httppost.addHeader("Content-Type", "application/json");
+ httppost.setEntity(new StringEntity(body, Consts.UTF_8));
+
+ try (final CloseableHttpResponse response =
httpclient.execute(httppost)) {
+ assertEquals(400, response.getStatusLine().getStatusCode());
+ }
+ }
+
+ @Test
+ public void shouldAcceptLanguageEmbeddedInScript() throws Exception {
+ // a with('language', ...) embedded in the script selects the script
engine. this uses a groovy-only
+ // construct (a closure), which only evaluates if the groovy engine
was selected from the embedded option.
+ final String body = "{ \"gremlin\": \"" + "g.with('language',
'gremlin-groovy').inject(1).map{it.get() + 1}" + "\"}";
+ final CloseableHttpClient httpclient = HttpClients.createDefault();
+ final HttpPost httppost = new
HttpPost(TestClientFactory.createURLString());
+ httppost.addHeader("Content-Type", "application/json");
+ httppost.setEntity(new StringEntity(body, Consts.UTF_8));
+
+ try (final CloseableHttpResponse response =
httpclient.execute(httppost)) {
+ assertEquals(200, response.getStatusLine().getStatusCode());
+ final String json = EntityUtils.toString(response.getEntity());
+ final JsonNode node = mapper.readTree(json);
+ assertEquals(2,
node.get("result").get(TOKEN_DATA).get(GraphSONTokens.VALUEPROP).get(0).get(GraphSONTokens.VALUEPROP).intValue());
+ }
+ }
+
+ @Test
+ public void shouldAcceptBulkResultsEmbeddedInScript() throws Exception {
+ // bulking applies only for gremlin-lang over GraphBinary. a
with('bulkResults', true) embedded in the script
+ // must enable it, observable via the bulked flag on the response.
+ final String gremlin = "g.with('bulkResults', true).inject(1, 2, 3, 2,
1)";
+ final GraphBinaryMessageSerializerV4 serializer = new
GraphBinaryMessageSerializerV4();
+ final ByteBuf serializedRequest = serializer.serializeRequestAsBinary(
+ RequestMessage.build(gremlin).create(), new
UnpooledByteBufAllocator(false));
+
+ final CloseableHttpClient httpclient = HttpClients.createDefault();
+ final HttpPost httppost = new
HttpPost(TestClientFactory.createURLString());
+ httppost.addHeader(HttpHeaders.CONTENT_TYPE,
Serializers.GRAPHBINARY_V4.getValue());
+ httppost.addHeader(HttpHeaders.ACCEPT,
Serializers.GRAPHBINARY_V4.getValue());
+ httppost.setEntity(new ByteArrayEntity(serializedRequest.array()));
+
+ try (final CloseableHttpResponse response =
httpclient.execute(httppost)) {
+ assertEquals(200, response.getStatusLine().getStatusCode());
+ final ResponseMessage responseMessage =
serializer.readChunk(toByteBuf(response.getEntity()), true);
+ assertTrue(responseMessage.getResult().isBulked());
+ }
+ }
+
@Test
public void shouldRespectCorsAllowedOrigins() throws Exception {
final CloseableHttpClient httpclient = HttpClients.createDefault();
diff --git
a/gremlin-tools/gremlin-benchmark/src/main/java/org/apache/tinkerpop/jsr223/GremlinScriptCheckerBenchmark.java
b/gremlin-tools/gremlin-benchmark/src/main/java/org/apache/tinkerpop/jsr223/GremlinScriptCheckerBenchmark.java
index 1ffe094c60..cc174d9d5d 100644
---
a/gremlin-tools/gremlin-benchmark/src/main/java/org/apache/tinkerpop/jsr223/GremlinScriptCheckerBenchmark.java
+++
b/gremlin-tools/gremlin-benchmark/src/main/java/org/apache/tinkerpop/jsr223/GremlinScriptCheckerBenchmark.java
@@ -33,8 +33,8 @@ import java.util.Optional;
public class GremlinScriptCheckerBenchmark extends AbstractBenchmarkBase {
@Benchmark
- public Optional<String> testParseRequestId() {
- return GremlinScriptChecker.parse("g.with('requestId',
'4F53FB59-CFC9-4984-B477-452073A352FD').with(true).V().out('knows')").getRequestId();
+ public Optional<Long> testParseTimeout() {
+ return GremlinScriptChecker.parse("g.with('timeoutMillis',
1000L).with(true).V().out('knows')").getTimeout();
}
@Benchmark
@@ -44,6 +44,6 @@ public class GremlinScriptCheckerBenchmark extends
AbstractBenchmarkBase {
@Benchmark
public GremlinScriptChecker.Result testParseAll() {
- return GremlinScriptChecker.parse("g.with('timeoutMillis',
1000L).with('materializeProperties', 'all').with('requestId',
'4F53FB59-CFC9-4984-B477-452073A352FD').with(true).V().out('knows')");
+ return GremlinScriptChecker.parse("g.with('timeoutMillis',
1000L).with('materializeProperties', 'all').with('language',
'gremlin-lang').with('batchSize', 100).with('bulkResults',
true).with(true).V().out('knows')");
}
}
