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

kenhuuu pushed a commit to branch master-http
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


The following commit(s) were added to refs/heads/master-http by this push:
     new 5ebc359e90 Add back evaluationTimeout and materializeProperties
5ebc359e90 is described below

commit 5ebc359e908bc80da803927d04af1130a053aa76
Author: Ken Hu <106191785+kenh...@users.noreply.github.com>
AuthorDate: Thu Apr 18 08:59:10 2024 -0700

    Add back evaluationTimeout and materializeProperties
    
    evaluationTimeout and materializeProperties should be allowed to be set
    from the request body as it affects how the server should execute the
    query.
---
 .../apache/tinkerpop/gremlin/driver/Client.java    |   8 +-
 .../tinkerpop/gremlin/driver/RequestOptions.java   |   6 +-
 .../apache/tinkerpop/gremlin/server/Context.java   |  11 +-
 .../server/handler/HttpGremlinEndpointHandler.java |   3 +-
 .../server/handler/HttpRequestMessageDecoder.java  |  19 +-
 .../server/GremlinServerHttpIntegrateTest.java     |  68 ++++++-
 .../gremlin/server/GremlinServerIntegrateTest.java | 224 ++++++++++-----------
 .../gremlin/util/message/RequestMessageV4.java     |  18 ++
 .../gremlin/util/message/RequestMessageV4Test.java |  26 +++
 9 files changed, 250 insertions(+), 133 deletions(-)

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 bc718945c0..1bf3c1dbb8 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
@@ -356,13 +356,13 @@ public abstract class Client {
                 .addChunkSize(batchSize);
 
         // apply settings if they were made available
-//        options.getTimeout().ifPresent(timeout -> 
request.add(Tokens.ARGS_EVAL_TIMEOUT, timeout));
+        options.getTimeout().ifPresent(timeout -> 
request.addTimeoutMillis(timeout));
         options.getParameters().ifPresent(params -> 
request.addBindings(params));
         options.getAliases().ifPresent(aliases -> {if (aliases.get("g") != 
null) request.addG(aliases.get("g")); });
 //        options.getOverrideRequestId().ifPresent(request::overrideRequestId);
 //        options.getUserAgent().ifPresent(userAgent -> 
request.addArg(Tokens.ARGS_USER_AGENT, userAgent));
         options.getLanguage().ifPresent(lang -> request.addLanguage(lang));
-//        options.getMaterializeProperties().ifPresent(mp -> 
request.addArg(Tokens.ARGS_MATERIALIZE_PROPERTIES, mp));
+        options.getMaterializeProperties().ifPresent(mp -> 
request.addMaterializeProperties(mp));
 
         return submitAsync(request.create());
     }
@@ -655,10 +655,10 @@ public abstract class Client {
 
                 // apply settings if they were made available
                 options.getBatchSize().ifPresent(batchSize -> 
request.addChunkSize(batchSize));
-//                options.getTimeout().ifPresent(timeout -> 
request.add(Tokens.ARGS_EVAL_TIMEOUT, timeout));
+                options.getTimeout().ifPresent(timeout -> 
request.addTimeoutMillis(timeout));
 //                
options.getOverrideRequestId().ifPresent(request::overrideRequestId);
 //                options.getUserAgent().ifPresent(userAgent -> 
request.add(Tokens.ARGS_USER_AGENT, userAgent));
-//                options.getMaterializeProperties().ifPresent(mp -> 
request.addArg(Tokens.ARGS_MATERIALIZE_PROPERTIES, mp));
+                options.getMaterializeProperties().ifPresent(mp -> 
request.addMaterializeProperties(mp));
 
                 return submitAsync(request.create());
             } catch (RuntimeException re) {
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 3f66e2fbb0..63d7dad91a 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
@@ -41,7 +41,7 @@ public final class RequestOptions {
     private final UUID overrideRequestId;
     private final String userAgent;
     private final String language;
-//    private final String materializeProperties;
+    private final String materializeProperties;
 
     private RequestOptions(final Builder builder) {
         this.aliases = builder.aliases;
@@ -51,7 +51,7 @@ public final class RequestOptions {
         this.overrideRequestId = builder.overrideRequestId;
         this.userAgent = builder.userAgent;
         this.language = builder.language;
-//        this.materializeProperties = builder.materializeProperties;
+        this.materializeProperties = builder.materializeProperties;
     }
 
     public Optional<UUID> getOverrideRequestId() {
@@ -82,7 +82,7 @@ public final class RequestOptions {
         return Optional.ofNullable(language);
     }
 
-//    public Optional<String> getMaterializeProperties() { return 
Optional.ofNullable(materializeProperties); }
+    public Optional<String> getMaterializeProperties() { return 
Optional.ofNullable(materializeProperties); }
 
     public static Builder build() {
         return new Builder();
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 26513f2646..88bd88e690 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
@@ -200,13 +200,15 @@ public class Context {
     private long determineTimeout() {
         // timeout override - handle both deprecated and newly named 
configuration. earlier logic should prevent
         // both configurations from being submitted at the same time
-        // evaluationTimeout RequestMessage argument was removed starting in 
4.0.
+        final Long timeoutMs = 
requestMessage.getField(Tokens.ARGS_EVAL_TIMEOUT);
+        final long seto = (null != timeoutMs) ? timeoutMs : 
settings.getEvaluationTimeout();
+
         // 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 = requestContentType == 
RequestContentType.SCRIPT ?
                 
GremlinScriptChecker.parse(gremlinArgument.toString()).getTimeout() : 
Optional.empty();
 
-        return timeoutDefinedInScript.orElse(settings.getEvaluationTimeout());
+        return timeoutDefinedInScript.orElse(seto);
     }
 
     private String determineMaterializeProperties() {
@@ -219,8 +221,11 @@ public class Context {
                         : Tokens.MATERIALIZE_PROPERTIES_ALL;
         }
 
+        final String materializeProperties = 
requestMessage.getField(Tokens.ARGS_MATERIALIZE_PROPERTIES);
         // all options except MATERIALIZE_PROPERTIES_TOKENS treated as 
MATERIALIZE_PROPERTIES_ALL
-        return Tokens.MATERIALIZE_PROPERTIES_ALL;
+        return 
Tokens.MATERIALIZE_PROPERTIES_TOKENS.equals(materializeProperties)
+                ? Tokens.MATERIALIZE_PROPERTIES_TOKENS
+                : Tokens.MATERIALIZE_PROPERTIES_ALL;
     }
 
     public void handleDetachment(final List<Object> aggregate) {
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 2196795250..906792f414 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
@@ -180,7 +180,8 @@ public class HttpGremlinEndpointHandler extends 
SimpleChannelInboundHandler<Requ
         final Timer.Context timerContext = evalOpTimer.time();
         // timeout override - handle both deprecated and newly named 
configuration. earlier logic should prevent
         // both configurations from being submitted at the same time
-        final long seto = requestCtx.getSettings().getEvaluationTimeout();
+        Long timeoutMs = requestMessage.getField(Tokens.ARGS_EVAL_TIMEOUT);
+        final long seto = (null != timeoutMs) ? timeoutMs : 
requestCtx.getSettings().getEvaluationTimeout();
 
         final FutureTask<Void> evalFuture = new FutureTask<>(() -> {
             requestCtx.setStartedResponse();
diff --git 
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java
 
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java
index cefc04f41f..1b261c3152 100644
--- 
a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java
+++ 
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java
@@ -204,6 +204,8 @@ public class HttpRequestMessageDecoder extends 
MessageToMessageDecoder<FullHttpR
         final JsonNode scriptNode = body.get(Tokens.ARGS_GREMLIN);
         if (null == scriptNode) throw new IllegalArgumentException("no gremlin 
script supplied");
 
+        final RequestMessageV4.Builder builder = 
RequestMessageV4.build(scriptNode.asText());
+
         final JsonNode bindingsNode = body.get(Tokens.ARGS_BINDINGS);
         if (bindingsNode != null && !bindingsNode.isObject())
             throw new IllegalArgumentException("bindings must be a Map");
@@ -211,20 +213,23 @@ public class HttpRequestMessageDecoder extends 
MessageToMessageDecoder<FullHttpR
         final Map<String, Object> bindings = new HashMap<>();
         if (bindingsNode != null)
             bindingsNode.fields().forEachRemaining(kv -> 
bindings.put(kv.getKey(), fromJsonNode(kv.getValue())));
+        builder.addBindings(bindings);
 
         final JsonNode gNode = body.get(Tokens.ARGS_G);
-        final String g = (null == gNode) ? null : gNode.asText();
+        if (null != gNode) builder.addG(gNode.asText());
 
         final JsonNode languageNode = body.get(Tokens.ARGS_LANGUAGE);
-        final String language = null == languageNode ? "gremlin-groovy" : 
languageNode.asText();
+        builder.addLanguage((null == languageNode) ? "gremlin-groovy" : 
languageNode.asText());
 
         final JsonNode chunkSizeNode = body.get(Tokens.ARGS_BATCH_SIZE);
-        final Integer chunkSize = null == chunkSizeNode ? null : 
chunkSizeNode.asInt();
+        if (null != chunkSizeNode) builder.addChunkSize(chunkSizeNode.asInt());
+
+        final JsonNode timeoutMsNode = body.get(Tokens.ARGS_EVAL_TIMEOUT);
+        if (null != timeoutMsNode) 
builder.addTimeoutMillis(timeoutMsNode.asLong());
+
+        final JsonNode matPropsNode = 
body.get(Tokens.ARGS_MATERIALIZE_PROPERTIES);
+        if (null != matPropsNode) 
builder.addMaterializeProperties(matPropsNode.asText());
 
-        final RequestMessageV4.Builder builder = 
RequestMessageV4.build(scriptNode.asText())
-                .addBindings(bindings).addLanguage(language);
-        if (null != g) builder.addG(g);
-        if (null != chunkSize) builder.addChunkSize(chunkSize);
         return builder.create();
     }
 
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 bd7e85170b..96b09abc22 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
@@ -30,11 +30,17 @@ import org.apache.http.client.config.RequestConfig;
 import org.apache.http.conn.EofSensorInputStream;
 import org.apache.http.entity.ByteArrayEntity;
 import org.apache.http.impl.io.ChunkedInputStream;
+import org.apache.tinkerpop.gremlin.driver.exception.ResponseException;
 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.dsl.graph.__;
 import 
org.apache.tinkerpop.gremlin.server.handler.HttpBasicAuthenticationHandler;
 import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
+import org.apache.tinkerpop.gremlin.util.Tokens;
+import org.apache.tinkerpop.gremlin.util.function.Lambda;
 import org.apache.tinkerpop.gremlin.util.message.RequestMessageV4;
 import org.apache.tinkerpop.gremlin.util.message.ResponseMessage;
+import org.apache.tinkerpop.gremlin.util.message.ResponseStatusCode;
 import org.apache.tinkerpop.gremlin.util.ser.GraphBinaryMessageSerializerV4;
 import org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV4;
 import 
org.apache.tinkerpop.gremlin.util.ser.GraphSONUntypedMessageSerializerV4;
@@ -70,7 +76,12 @@ import java.util.concurrent.Future;
 import java.util.concurrent.ScheduledExecutorService;
 import java.util.concurrent.TimeUnit;
 
+import static 
org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource.traversal;
 import static 
org.apache.tinkerpop.gremlin.server.handler.HttpRequestIdHandler.REQUEST_ID_HEADER_NAME;
+import static org.apache.tinkerpop.gremlin.util.Tokens.ARGS_EVAL_TIMEOUT;
+import static 
org.apache.tinkerpop.gremlin.util.Tokens.ARGS_MATERIALIZE_PROPERTIES;
+import static 
org.apache.tinkerpop.gremlin.util.Tokens.MATERIALIZE_PROPERTIES_ALL;
+import static 
org.apache.tinkerpop.gremlin.util.Tokens.MATERIALIZE_PROPERTIES_TOKENS;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
 import static org.hamcrest.core.IsInstanceOf.instanceOf;
@@ -78,7 +89,10 @@ import static 
org.hamcrest.core.StringRegularExpression.matchesRegex;
 import static org.hamcrest.core.StringStartsWith.startsWith;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
 import static org.junit.Assert.assertTrue;
+import static org.junit.Assert.fail;
 
 /**
  * Integration tests for server-side settings and processing.
@@ -1058,7 +1072,7 @@ public class GremlinServerHttpIntegrateTest extends 
AbstractGremlinServerIntegra
         firstPost.setEntity(new 
StringEntity("{\"gremlin\":\"g.V().repeat(__.out()).until(__.outE().count().is(0)).iterate()\"}",
 Consts.UTF_8));
         // Add a shorter timeout to the second query to ensure that its 
timeout is less than the first query's running time.
         final HttpPost secondPost = new 
HttpPost(TestClientFactory.createURLString());
-        secondPost.setEntity(new 
StringEntity("{\"gremlin\":\"g.with('evaluationTimeout',1000).V().repeat(__.out()).until(__.outE().count().is(0)).iterate()\"}",
 Consts.UTF_8));
+        secondPost.setEntity(new 
StringEntity("{\"gremlin\":\"g.V().repeat(__.out()).until(__.outE().count().is(0)).iterate()\"}",
 Consts.UTF_8));
 
         final Callable<Integer> firstQueryWrapper = () -> {
             try (final CloseableHttpResponse response = 
firstClient.execute(firstPost)) {
@@ -1416,6 +1430,58 @@ public class GremlinServerHttpIntegrateTest extends 
AbstractGremlinServerIntegra
         }
     }
 
+    @Test
+    public void shouldAcceptTimeoutInRequestBody() throws Exception {
+        final String body = "{ \"gremlin\": \"" + "Thread.sleep(5000)" + 
"\",\"language\":\"gremlin-groovy\",\"" + ARGS_EVAL_TIMEOUT + "\":\"100\"}";
+        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 shouldAcceptMaterializePropertiesAllInRequestBody() throws 
Exception {
+        final String body = "{ \"gremlin\": \"" + "gmodern.V().limit(1)" + 
"\",\"language\":\"gremlin-groovy\",\""
+                + ARGS_MATERIALIZE_PROPERTIES + "\":\"" + 
MATERIALIZE_PROPERTIES_ALL + "\"}";
+        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);
+            
assertNotNull(node.get("result").get(GraphSONTokens.VALUEPROP).get(0).get(GraphSONTokens.VALUEPROP).get(GraphSONTokens.PROPERTIES));
+        }
+    }
+
+    @Test
+    public void shouldAcceptMaterializePropertiesTokensInRequestBody() throws 
Exception {
+        final String body = "{ \"gremlin\": \"" + "gmodern.V().limit(1)" + 
"\",\"language\":\"gremlin-groovy\",\""
+                + ARGS_MATERIALIZE_PROPERTIES + "\":\"" + 
MATERIALIZE_PROPERTIES_TOKENS + "\"}";
+        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(GraphSONTokens.VALUEPROP).get(0).get(GraphSONTokens.VALUEPROP).get(GraphSONTokens.PROPERTIES));
+        }
+    }
+
     private static ByteBuf toByteBuf(final HttpEntity httpEntity) throws 
IOException {
         final byte[] asArray = EntityUtils.toByteArray(httpEntity);
         return Unpooled.wrappedBuffer(asArray);
diff --git 
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
 
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
index 8f5d77efd5..5e530634b2 100644
--- 
a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
+++ 
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
@@ -77,9 +77,11 @@ import static 
org.apache.tinkerpop.gremlin.groovy.jsr223.GroovyCompilerGremlinPl
 import static 
org.apache.tinkerpop.gremlin.process.remote.RemoteConnection.GREMLIN_REMOTE;
 import static 
org.apache.tinkerpop.gremlin.process.remote.RemoteConnection.GREMLIN_REMOTE_CONNECTION_CLASS;
 import static 
org.apache.tinkerpop.gremlin.process.traversal.AnonymousTraversalSource.traversal;
+import static org.apache.tinkerpop.gremlin.util.Tokens.ARGS_EVAL_TIMEOUT;
 import static org.hamcrest.CoreMatchers.containsString;
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.AllOf.allOf;
 import static org.hamcrest.core.IsInstanceOf.instanceOf;
 import static org.hamcrest.core.StringStartsWith.startsWith;
 import static org.junit.Assert.assertEquals;
@@ -172,10 +174,9 @@ public class GremlinServerIntegrateTest extends 
AbstractGremlinServerIntegration
             case 
"shouldReturnInvalidRequestArgsWhenBindingCountExceedsAllowable":
                 settings.maxParameters = 1;
                 break;
-//                TODO: evalTimeout needs be part of script now
-//            case "shouldTimeOutRemoteTraversal":
-//                settings.evaluationTimeout = 500;
-//                break;
+            case "shouldTimeOutRemoteTraversal":
+                settings.evaluationTimeout = 500;
+                break;
             case "shouldBlowTheWorkQueueSize":
                 settings.gremlinPool = 1;
                 settings.maxWorkQueueSize = 1;
@@ -334,102 +335,99 @@ public class GremlinServerIntegrateTest extends 
AbstractGremlinServerIntegration
         g.close();
     }
 
-//    TODO: evalTimeout needs be part of script now
-//    @Test
-//    public void shouldTimeOutRemoteTraversal() throws Exception {
-//        final GraphTraversalSource g = traversal().withRemote(conf);
-//
-//        try {
-//            // tests sleeping thread
-//            
g.inject(1).sideEffect(Lambda.consumer("Thread.sleep(20000)")).iterate();
-//            fail("This traversal should have timed out");
-//        } catch (Exception ex) {
-//            final Throwable t = ex.getCause();
-//            assertThat(t, instanceOf(ResponseException.class));
-//            assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, 
((ResponseException) t).getResponseStatusCode());
-//        }
-//
-//        // make a graph with a cycle in it to force a long run traversal
-//        
graphGetter.get().traversal().addV("person").as("p").addE("self").to("p").iterate();
-//
-//        try {
-//            // tests an "unending" traversal
-//            g.V().repeat(__.out()).until(__.outE().count().is(0)).iterate();
-//            fail("This traversal should have timed out");
-//        } catch (Exception ex) {
-//            final Throwable t = ex.getCause();
-//            assertThat(t, instanceOf(ResponseException.class));
-//            assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, 
((ResponseException) t).getResponseStatusCode());
-//        }
-//
-//        g.close();
-//    }
+    @Test
+    public void shouldTimeOutRemoteTraversal() throws Exception {
+        final GraphTraversalSource g = traversal().withRemote(conf);
 
-//    TODO: driver can't set EvalTimeout anymore in RequestMessage so this 
won't work.
-//    @Test
-//    public void shouldTimeOutRemoteTraversalWithPerRequestOption() throws 
Exception {
-//        final GraphTraversalSource g = traversal().withRemote(conf);
-//
-//        try {
-//            // tests sleeping thread
-//            g.with(ARGS_EVAL_TIMEOUT, 
500L).inject(1).sideEffect(Lambda.consumer("Thread.sleep(10000)")).iterate();
-//            fail("This traversal should have timed out");
-//        } catch (Exception ex) {
-//            final Throwable t = ex.getCause();
-//            assertThat(t, instanceOf(ResponseException.class));
-//            assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, 
((ResponseException) t).getResponseStatusCode());
-//        }
-//
-//        // make a graph with a cycle in it to force a long run traversal
-//        
graphGetter.get().traversal().addV("person").as("p").addE("self").to("p").iterate();
-//
-//        try {
-//            // tests an "unending" traversal
-//            g.with(ARGS_EVAL_TIMEOUT, 
500L).V().repeat(__.out()).until(__.outE().count().is(0)).iterate();
-//            fail("This traversal should have timed out");
-//        } catch (Exception ex) {
-//            final Throwable t = ex.getCause();
-//            assertThat(t, instanceOf(ResponseException.class));
-//            assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, 
((ResponseException) t).getResponseStatusCode());
-//        }
-//
-//        g.close();
-//    }
+        try {
+            // tests sleeping thread
+            
g.inject(1).sideEffect(Lambda.consumer("Thread.sleep(10000)")).iterate();
+            fail("This traversal should have timed out");
+        } catch (Exception ex) {
+            final Throwable t = ex.getCause();
+            assertThat(t, instanceOf(ResponseException.class));
+            assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, 
((ResponseException) t).getResponseStatusCode());
+        }
 
-//    TODO: can't set evalTimeout anymore
-//    @Test
-//    public void shouldProduceProperExceptionOnTimeout() throws Exception {
-//        final Cluster cluster = TestClientFactory.open();
-//        final Client client = cluster.connect(name.getMethodName());
-//
-//        boolean success = false;
-//        // Run a short test script a few times with progressively longer 
timeouts.
-//        // Each submissions should either succeed or fail with a timeout.
-//        // Note: the range of timeouts is intended to cover the case when 
the script finishes at about the
-//        // same time when the timeout occurs. In this situation either a 
timeout response or a successful
-//        // response is acceptable, however no other processing errors should 
occur.
-//        // Note: the timeout of 30 ms is generally sufficient for running a 
simple groovy script, so using longer
-//        // timeouts are not likely to results in a success/timeout response 
collision, which is the purpose
-//        // of this test.
-//        // Note: this test may have a false negative result, but a failure  
would indicate a real problem.
-//        for(int i = 0; i < 30; i++) {
-//            int timeout = 1 + i;
-//            overrideEvaluationTimeout(timeout);
-//
-//            try {
-//                client.submit("x = 1 + 1").all().get().get(0).getInt();
-//                success = true;
-//            } catch (Exception ex) {
-//                final Throwable t = ex.getCause();
-//                assertThat("Unexpected exception with script evaluation 
timeout: " + timeout, t, instanceOf(ResponseException.class));
-//                assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, 
((ResponseException) t).getResponseStatusCode());
-//            }
-//        }
-//
-//        assertTrue("Some script submissions should succeed", success);
-//
-//        cluster.close();
-//    }
+        // make a graph with a cycle in it to force a long run traversal
+        
graphGetter.get().traversal().addV("person").as("p").addE("self").to("p").iterate();
+
+        try {
+            // tests an "unending" traversal
+            g.V().repeat(__.out()).until(__.outE().count().is(0)).iterate();
+            fail("This traversal should have timed out");
+        } catch (Exception ex) {
+            final Throwable t = ex.getCause();
+            assertThat(t, instanceOf(ResponseException.class));
+            assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, 
((ResponseException) t).getResponseStatusCode());
+        }
+
+        g.close();
+    }
+
+    @Test
+    public void shouldTimeOutRemoteTraversalWithPerRequestOption() throws 
Exception {
+        final GraphTraversalSource g = traversal().withRemote(conf);
+
+        try {
+            // tests sleeping thread
+            g.with(ARGS_EVAL_TIMEOUT, 
500L).inject(1).sideEffect(Lambda.consumer("Thread.sleep(10000)")).iterate();
+            fail("This traversal should have timed out");
+        } catch (Exception ex) {
+            final Throwable t = ex.getCause();
+            assertThat(t, instanceOf(ResponseException.class));
+            assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, 
((ResponseException) t).getResponseStatusCode());
+        }
+
+        // make a graph with a cycle in it to force a long run traversal
+        
graphGetter.get().traversal().addV("person").as("p").addE("self").to("p").iterate();
+
+        try {
+            // tests an "unending" traversal
+            g.with(ARGS_EVAL_TIMEOUT, 
500L).V().repeat(__.out()).until(__.outE().count().is(0)).iterate();
+            fail("This traversal should have timed out");
+        } catch (Exception ex) {
+            final Throwable t = ex.getCause();
+            assertThat(t, instanceOf(ResponseException.class));
+            assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, 
((ResponseException) t).getResponseStatusCode());
+        }
+
+        g.close();
+    }
+
+    @Test
+    public void shouldProduceProperExceptionOnTimeout() throws Exception {
+        final Cluster cluster = TestClientFactory.open();
+        final Client client = cluster.connect(name.getMethodName());
+
+        boolean success = false;
+        // Run a short test script a few times with progressively longer 
timeouts.
+        // Each submissions should either succeed or fail with a timeout.
+        // Note: the range of timeouts is intended to cover the case when the 
script finishes at about the
+        // same time when the timeout occurs. In this situation either a 
timeout response or a successful
+        // response is acceptable, however no other processing errors should 
occur.
+        // Note: the timeout of 30 ms is generally sufficient for running a 
simple groovy script, so using longer
+        // timeouts are not likely to results in a success/timeout response 
collision, which is the purpose
+        // of this test.
+        // Note: this test may have a false negative result, but a failure  
would indicate a real problem.
+        for(int i = 0; i < 30; i++) {
+            int timeout = 1 + i;
+            overrideEvaluationTimeout(timeout);
+
+            try {
+                client.submit("x = 1 + 1").all().get().get(0).getInt();
+                success = true;
+            } catch (Exception ex) {
+                final Throwable t = ex.getCause();
+                assertThat("Unexpected exception with script evaluation 
timeout: " + timeout, t, instanceOf(ResponseException.class));
+                assertEquals(ResponseStatusCode.SERVER_ERROR_TIMEOUT, 
((ResponseException) t).getResponseStatusCode());
+            }
+        }
+
+        assertTrue("Some script submissions should succeed", success);
+
+        cluster.close();
+    }
 
     @Test
     public void shouldUseBaseScript() throws Exception {
@@ -719,22 +717,20 @@ public class GremlinServerIntegrateTest extends 
AbstractGremlinServerIntegration
         }
     }
 
-    // TODO: evalTimeout no longer set in RequestMessage
-//    @Test
-//    @SuppressWarnings("unchecked")
-//    public void shouldReceiveFailureTimeOutOnEvalUsingOverride() throws 
Exception {
-//        try (SimpleClient client = 
TestClientFactory.createWebSocketClient()) {
-//            final RequestMessageV4 msg = RequestMessageV4.build("eval")
-//                    .addArg(Tokens.ARGS_EVAL_TIMEOUT, 100L)
-//                    .addArg(Tokens.ARGS_GREMLIN, 
"Thread.sleep(3000);'some-stuff-that-should not return'")
-//                    .create();
-//            final List<ResponseMessage> responses = client.submit(msg);
-//            assertThat(responses.get(0).getStatus().getMessage(), 
allOf(startsWith("Evaluation exceeded"), containsString("100 ms")));
-//
-//            // validate that we can still send messages to the server
-//            assertEquals(2, ((List<Integer>) 
client.submit("1+1").get(0).getResult().getData()).get(0).intValue());
-//        }
-//    }
+    @Test
+    @SuppressWarnings("unchecked")
+    public void shouldReceiveFailureTimeOutOnEvalUsingOverride() throws 
Exception {
+        try (SimpleClient client = TestClientFactory.createSimpleHttpClient()) 
{
+            final RequestMessageV4 msg = 
RequestMessageV4.build("Thread.sleep(3000);'some-stuff-that-should not return'")
+                    .addTimeoutMillis(100L)
+                    .create();
+            final List<ResponseMessage> responses = client.submit(msg);
+            assertThat(responses.get(0).getStatus().getMessage(), 
allOf(startsWith("Evaluation exceeded"), containsString("100 ms")));
+
+            // validate that we can still send messages to the server
+            assertEquals(2, ((List<Integer>) 
client.submit("1+1").get(0).getResult().getData()).get(0).intValue());
+        }
+    }
 
     @Test
     public void shouldReceiveFailureTimeOutOnScriptEvalOfOutOfControlLoop() 
throws Exception {
diff --git 
a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4.java
 
b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4.java
index 65c2b0029b..a0afeb5779 100644
--- 
a/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4.java
+++ 
b/gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4.java
@@ -181,6 +181,24 @@ public final class RequestMessageV4 {
             return this;
         }
 
+        public Builder addMaterializeProperties(final String materializeProps) 
{
+            Objects.requireNonNull(materializeProps, "materializeProps 
argument cannot be null.");
+            if (!materializeProps.equals(Tokens.MATERIALIZE_PROPERTIES_TOKENS) 
&& !materializeProps.equals(Tokens.MATERIALIZE_PROPERTIES_ALL)) {
+                throw new IllegalArgumentException("materializeProperties 
argument must be either token or all.");
+            }
+
+            this.fields.put(Tokens.ARGS_MATERIALIZE_PROPERTIES, 
materializeProps);
+            return this;
+        }
+
+        public Builder addTimeoutMillis(final long timeout) {
+            Objects.requireNonNull(timeout, "timeout argument cannot be 
null.");
+            if (timeout < 0) throw new IllegalArgumentException("timeout 
argument cannot be negative.");
+
+            this.fields.put(Tokens.ARGS_EVAL_TIMEOUT, timeout);
+            return this;
+        }
+
         /**
          * Create the request message given the settings provided to the 
{@link Builder}.
          */
diff --git 
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4Test.java
 
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4Test.java
index 5b17aabde6..d121524f72 100644
--- 
a/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4Test.java
+++ 
b/gremlin-util/src/test/java/org/apache/tinkerpop/gremlin/util/message/RequestMessageV4Test.java
@@ -83,6 +83,32 @@ public class RequestMessageV4Test {
         assertEquals(g, msg.getField(Tokens.ARGS_G));
     }
 
+    @Test
+    public void shouldSetTimeout() {
+        final long timeout = 101L;
+        final RequestMessageV4 msg = 
RequestMessageV4.build("g").addTimeoutMillis(timeout).create();
+        assertEquals(timeout, (long) msg.getField(Tokens.ARGS_EVAL_TIMEOUT));
+    }
+
+    @Test
+    public void shouldSetMaterializeProperties() {
+        final RequestMessageV4 msgWithAll = 
RequestMessageV4.build("g").addMaterializeProperties(Tokens.MATERIALIZE_PROPERTIES_ALL).create();
+        assertEquals(Tokens.MATERIALIZE_PROPERTIES_ALL, 
msgWithAll.getField(Tokens.ARGS_MATERIALIZE_PROPERTIES));
+
+        final RequestMessageV4 msgWithTokens = 
RequestMessageV4.build("g").addMaterializeProperties(Tokens.MATERIALIZE_PROPERTIES_TOKENS).create();
+        assertEquals(Tokens.MATERIALIZE_PROPERTIES_TOKENS, 
msgWithTokens.getField(Tokens.ARGS_MATERIALIZE_PROPERTIES));
+    }
+
+    @Test
+    public void shouldErrorSettingMaterializePropertiesWithInvalidValue() {
+        try {
+            final RequestMessageV4 msgWithTokens = 
RequestMessageV4.build("g").addMaterializeProperties("notToken").create();
+            fail("RequestMessage shouldn't accept notToken for 
materializeProperties.");
+        } catch (Exception e) {
+            assertTrue(e.getMessage().contains("materializeProperties argument 
must be either token or all"));
+        }
+    }
+
     @Test
     public void shouldGetFields() {
         final String g = "gmodern";


Reply via email to