You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2018/08/07 16:54:47 UTC
[1/9] tinkerpop git commit: TINKERPOP-2005 Reject multiple final
responses in AbstractEvalOpProcessor
Repository: tinkerpop
Updated Branches:
refs/heads/tp33 2cf551f08 -> fa7a7f613
TINKERPOP-2005 Reject multiple final responses in AbstractEvalOpProcessor
Add isFinalResponse() getter to ResponseStatusCode
Introduce ResponseHandlerContext to allow tracking the final response
status per request message.
Update AbstractOpProcessor, AbstractEvalOpProcessor and related classes
to write response messages through ResponseHandlerContext methods as
opposed to ChannelHandlerContext methods.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/f592e344
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/f592e344
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/f592e344
Branch: refs/heads/tp33
Commit: f592e3446c84e9398e242a072cdfec64025e9566
Parents: f9ad72a
Author: Dmitri Bourlatchkov <dm...@datastax.com>
Authored: Fri Jul 27 15:54:06 2018 -0400
Committer: Dmitri Bourlatchkov <dm...@datastax.com>
Committed: Fri Jul 27 15:54:06 2018 -0400
----------------------------------------------------------------------
.../driver/message/ResponseStatusCode.java | 7 ++
.../driver/message/ResponseStatusCodeTest.java | 35 ++++++
.../gremlin/server/ResponseHandlerContext.java | 80 ++++++++++++++
.../server/op/AbstractEvalOpProcessor.java | 17 +--
.../gremlin/server/op/AbstractOpProcessor.java | 34 +++++-
.../AbstractGremlinServerIntegrationTest.java | 9 +-
.../server/GremlinServerIntegrateTest.java | 51 +++++++++
.../server/ResponseHandlerContextTest.java | 110 +++++++++++++++++++
.../server/op/AbstractOpProcessorTest.java | 73 ++++++++++++
9 files changed, 401 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f592e344/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCode.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCode.java b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCode.java
index 3348107..f4e7f65 100644
--- a/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCode.java
+++ b/gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCode.java
@@ -123,4 +123,11 @@ public enum ResponseStatusCode {
public boolean isSuccess() {
return String.valueOf(this.value).startsWith("2");
}
+
+ /**
+ * Indicates whether the status code can only be used in the last response for a particular request.
+ */
+ public boolean isFinalResponse() {
+ return this != PARTIAL_CONTENT && this != AUTHENTICATE;
+ }
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f592e344/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCodeTest.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCodeTest.java b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCodeTest.java
new file mode 100644
index 0000000..a231489
--- /dev/null
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCodeTest.java
@@ -0,0 +1,35 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.tinkerpop.gremlin.driver.message;
+
+import org.junit.Test;
+
+import static org.junit.Assert.*;
+
+public class ResponseStatusCodeTest {
+
+ @Test
+ public void shouldIndicateFinalMessagesStatusCodes() {
+ assertFalse(ResponseStatusCode.AUTHENTICATE.isFinalResponse());
+ assertFalse(ResponseStatusCode.PARTIAL_CONTENT.isFinalResponse());
+ assertTrue(ResponseStatusCode.SUCCESS.isFinalResponse());
+ assertTrue(ResponseStatusCode.SERVER_ERROR_TIMEOUT.isFinalResponse());
+ }
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f592e344/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
new file mode 100644
index 0000000..fff4480
--- /dev/null
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.server;
+
+import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode;
+
+import java.util.concurrent.atomic.AtomicBoolean;
+
+/**
+ * A context for asynchronously writing response messages related to a particular request.
+ * <p>The "write" methods of this class ensure that at most one {@link ResponseStatusCode#isFinalResponse() final}
+ * response message is written to the underlying channel. Attempts to write more than one final response message will
+ * result in an {@link IllegalStateException}.</p>
+ * <p>Note: an object of this class should be used instead of writing to the channel directly when multiple threads
+ * are expected to produce final response messages concurrently. Callers must ensure that the same
+ * {@link ResponseHandlerContext} is used by all threads writing response messages for the same request.</p>
+ *
+ * @author Dmitri Bourlatchkov
+ */
+public class ResponseHandlerContext {
+
+ private final Context context;
+ private final AtomicBoolean finalResponseWritten = new AtomicBoolean();
+
+ public ResponseHandlerContext(Context context) {
+ this.context = context;
+ }
+
+ public Context getContext() {
+ return context;
+ }
+
+ /**
+ * Writes a response message to the underlying channel while ensuring that at most one
+ * {@link ResponseStatusCode#isFinalResponse() final} response is written.
+ * <p>Note: this method should be used instead of writing to the channel directly when multiple threads
+ * are expected to produce response messages concurrently.</p>
+ * <p>Attempts to write more than one final response message will result in an {@link IllegalStateException}.</p>
+ * @see #writeAndFlush(ResponseStatusCode, Object)
+ */
+ public void writeAndFlush(ResponseMessage message) {
+ writeAndFlush(message.getStatus().getCode(), message);
+ }
+
+ /**
+ * Writes a response message to the underlying channel while ensuring that at most one
+ * {@link ResponseStatusCode#isFinalResponse() final} response is written.
+ * <p>The caller must make sure that the provided response status code matches the content of the message.</p>
+ * <p>Note: this method should be used instead of writing to the channel directly when multiple threads
+ * are expected to produce response messages concurrently.</p>
+ * <p>Attempts to write more than one final response message will result in an {@link IllegalStateException}.</p>
+ * @see #writeAndFlush(ResponseMessage)
+ */
+ public void writeAndFlush(ResponseStatusCode code, Object responseMessage) {
+ final boolean messageIsFinal = code.isFinalResponse();
+ if(!finalResponseWritten.compareAndSet(false, messageIsFinal)) {
+ final String errorMessage = String.format("Another final response message was already written for request %s", context.getRequestMessage().getRequestId());
+ throw new IllegalStateException(errorMessage);
+ }
+
+ context.getChannelHandlerContext().writeAndFlush(responseMessage);
+ }
+}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f592e344/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
index 5c43b4d..6ff0452 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
@@ -36,6 +36,7 @@ import org.apache.tinkerpop.gremlin.structure.Column;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.server.Context;
import org.apache.tinkerpop.gremlin.server.GremlinServer;
+import org.apache.tinkerpop.gremlin.server.ResponseHandlerContext;
import org.apache.tinkerpop.gremlin.server.Settings;
import org.apache.tinkerpop.gremlin.server.util.MetricManager;
import org.apache.tinkerpop.gremlin.util.function.ThrowingConsumer;
@@ -245,6 +246,8 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
final long seto = args.containsKey(Tokens.ARGS_SCRIPT_EVAL_TIMEOUT) ?
Long.parseLong(args.get(Tokens.ARGS_SCRIPT_EVAL_TIMEOUT).toString()) : settings.scriptEvaluationTimeout;
+ ResponseHandlerContext rhc = new ResponseHandlerContext(context);
+
final GremlinExecutor.LifeCycle lifeCycle = GremlinExecutor.LifeCycle.build()
.scriptEvaluationTimeoutOverride(seto)
.afterFailure((b,t) -> {
@@ -265,7 +268,7 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
logger.debug("Preparing to iterate results from - {} - in thread [{}]", msg, Thread.currentThread().getName());
try {
- handleIterator(context, itty);
+ handleIterator(rhc, itty);
} catch (Exception ex) {
if (managedTransactionsForRequest) attemptRollback(msg, context.getGraphManager(), settings.strictTransactionManagement);
@@ -282,25 +285,25 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
if (t != null) {
if (t instanceof OpProcessorException) {
- ctx.writeAndFlush(((OpProcessorException) t).getResponseMessage());
+ rhc.writeAndFlush(((OpProcessorException) t).getResponseMessage());
} else if (t instanceof TimedInterruptTimeoutException) {
// occurs when the TimedInterruptCustomizerProvider is in play
final String errorMessage = String.format("A timeout occurred within the script during evaluation of [%s] - consider increasing the limit given to TimedInterruptCustomizerProvider", msg);
logger.warn(errorMessage);
- ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
+ rhc.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
.statusMessage("Timeout during script evaluation triggered by TimedInterruptCustomizerProvider")
.statusAttributeException(t).create());
} else if (t instanceof org.apache.tinkerpop.gremlin.groovy.jsr223.TimedInterruptTimeoutException) {
// occurs when the TimedInterruptCustomizerProvider is in play
final String errorMessage = String.format("A timeout occurred within the script during evaluation of [%s] - consider increasing the limit given to TimedInterruptCustomizerProvider", msg);
logger.warn(errorMessage);
- ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
+ rhc.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
.statusMessage("Timeout during script evaluation triggered by TimedInterruptCustomizerProvider")
.statusAttributeException(t).create());
} else if (t instanceof TimeoutException) {
final String errorMessage = String.format("Script evaluation exceeded the configured threshold for request [%s]", msg);
logger.warn(errorMessage, t);
- ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
+ rhc.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
.statusMessage(t.getMessage())
.statusAttributeException(t).create());
} else {
@@ -314,12 +317,12 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
((MultipleCompilationErrorsException) t).getErrorCollector().getErrorCount() == 1) {
final String errorMessage = String.format("The Gremlin statement that was submitted exceed the maximum compilation size allowed by the JVM, please split it into multiple smaller statements - %s", trimMessage(msg));
logger.warn(errorMessage);
- ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION)
+ rhc.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION)
.statusMessage(errorMessage)
.statusAttributeException(t).create());
} else {
logger.warn(String.format("Exception processing a script on request [%s].", msg), t);
- ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION)
+ rhc.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_SCRIPT_EVALUATION)
.statusMessage(t.getMessage())
.statusAttributeException(t).create());
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f592e344/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
index 8899bb5..1263c81 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
@@ -29,6 +29,7 @@ import org.apache.tinkerpop.gremlin.driver.ser.MessageTextSerializer;
import org.apache.tinkerpop.gremlin.server.Context;
import org.apache.tinkerpop.gremlin.server.GraphManager;
import org.apache.tinkerpop.gremlin.server.OpProcessor;
+import org.apache.tinkerpop.gremlin.server.ResponseHandlerContext;
import org.apache.tinkerpop.gremlin.server.Settings;
import org.apache.tinkerpop.gremlin.server.handler.Frame;
import org.apache.tinkerpop.gremlin.server.handler.StateKey;
@@ -72,8 +73,19 @@ public abstract class AbstractOpProcessor implements OpProcessor {
* @param context The Gremlin Server {@link Context} object containing settings, request message, etc.
* @param itty The result to iterator
* @throws TimeoutException if the time taken to serialize the entire result set exceeds the allowable time.
+ * @see #handleIterator(ResponseHandlerContext, Iterator)
*/
protected void handleIterator(final Context context, final Iterator itty) throws TimeoutException, InterruptedException {
+ handleIterator(new ResponseHandlerContext(context), itty);
+ }
+
+ /**
+ * A variant of {@link #handleIterator(Context, Iterator)} that is suitable for use in situations when mutiple
+ * threads may produce {@link ResponseStatusCode#isFinalResponse() final} response messages concurrently.
+ * @see #handleIterator(Context, Iterator)
+ */
+ protected void handleIterator(final ResponseHandlerContext rhc, final Iterator itty) throws TimeoutException, InterruptedException {
+ final Context context = rhc.getContext();
final ChannelHandlerContext ctx = context.getChannelHandlerContext();
final RequestMessage msg = context.getRequestMessage();
final Settings settings = context.getSettings();
@@ -90,7 +102,7 @@ public abstract class AbstractOpProcessor implements OpProcessor {
// as there is nothing left to iterate if we are transaction managed then we should execute a
// commit here before we send back a NO_CONTENT which implies success
if (managedTransactionsForRequest) attemptCommit(msg, context.getGraphManager(), settings.strictTransactionManagement);
- ctx.writeAndFlush(ResponseMessage.build(msg)
+ rhc.writeAndFlush(ResponseMessage.build(msg)
.code(ResponseStatusCode.NO_CONTENT)
.create());
return;
@@ -143,7 +155,7 @@ public abstract class AbstractOpProcessor implements OpProcessor {
// thread that processed the eval of the script so, we have to push serialization down into that
Frame frame = null;
try {
- frame = makeFrame(ctx, msg, serializer, useBinary, aggregate, code, generateMetaData(ctx, msg, code, itty));
+ frame = makeFrame(rhc, msg, serializer, useBinary, aggregate, code, generateMetaData(ctx, msg, code, itty));
} catch (Exception ex) {
// a frame may use a Bytebuf which is a countable release - if it does not get written
// downstream it needs to be released here
@@ -191,7 +203,7 @@ public abstract class AbstractOpProcessor implements OpProcessor {
// required then it will be 100% complete before the client receives it. the "frame" at this point
// should have completely detached objects from the transaction (i.e. serialization has occurred)
// so a new one should not be opened on the flush down the netty pipeline
- ctx.writeAndFlush(frame);
+ rhc.writeAndFlush(code, frame);
}
} else {
// don't keep triggering this warning over and over again for the same request
@@ -252,15 +264,29 @@ public abstract class AbstractOpProcessor implements OpProcessor {
/**
* @deprecated As of release 3.2.2, replaced by {@link #makeFrame(ChannelHandlerContext, RequestMessage, MessageSerializer, boolean, List, ResponseStatusCode, Map)}.
*/
+ @Deprecated
protected static Frame makeFrame(final ChannelHandlerContext ctx, final RequestMessage msg,
final MessageSerializer serializer, final boolean useBinary, final List<Object> aggregate,
final ResponseStatusCode code) throws Exception {
return makeFrame(ctx, msg, serializer, useBinary, aggregate, code, Collections.emptyMap());
}
+ /**
+ * Caution: {@link #makeFrame(ResponseHandlerContext, RequestMessage, MessageSerializer, boolean, List, ResponseStatusCode, Map)}
+ * should be used instead of this method whenever a {@link ResponseHandlerContext} is available.
+ */
protected static Frame makeFrame(final ChannelHandlerContext ctx, final RequestMessage msg,
final MessageSerializer serializer, final boolean useBinary, final List<Object> aggregate,
final ResponseStatusCode code, final Map<String,Object> responseMetaData) throws Exception {
+ Context context = new Context(msg, ctx, null, null, null, null); // dummy context, good only for writing response messages to the channel
+ ResponseHandlerContext rhc = new ResponseHandlerContext(context);
+ return makeFrame(rhc, msg, serializer, useBinary, aggregate, code, responseMetaData);
+ }
+
+ protected static Frame makeFrame(final ResponseHandlerContext rhc, final RequestMessage msg,
+ final MessageSerializer serializer, final boolean useBinary, final List<Object> aggregate,
+ final ResponseStatusCode code, final Map<String,Object> responseMetaData) throws Exception {
+ final ChannelHandlerContext ctx = rhc.getContext().getChannelHandlerContext();
try {
if (useBinary) {
return new Frame(serializer.serializeResponseAsBinary(ResponseMessage.build(msg)
@@ -283,7 +309,7 @@ public abstract class AbstractOpProcessor implements OpProcessor {
.statusMessage(errorMessage)
.statusAttributeException(ex)
.code(ResponseStatusCode.SERVER_ERROR_SERIALIZATION).create();
- ctx.writeAndFlush(error);
+ rhc.writeAndFlush(error);
throw ex;
}
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f592e344/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
index 7c124f0..a8a8853 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
@@ -39,6 +39,7 @@ import static org.junit.Assume.assumeThat;
*/
public abstract class AbstractGremlinServerIntegrationTest {
protected GremlinServer server;
+ protected Settings overriddenSettings;
private final static String epollOption = "gremlin.server.epoll";
private static final boolean GREMLIN_SERVER_EPOLL = "true".equalsIgnoreCase(System.getProperty(epollOption));
private static final Logger logger = LoggerFactory.getLogger(AbstractGremlinServerIntegrationTest.class);
@@ -87,13 +88,13 @@ public abstract class AbstractGremlinServerIntegrationTest {
public void startServer() throws Exception {
final InputStream stream = getSettingsInputStream();
final Settings settings = Settings.read(stream);
- final Settings overridenSettings = overrideSettings(settings);
- ServerTestHelper.rewritePathsInGremlinServerSettings(overridenSettings);
+ overriddenSettings = overrideSettings(settings);
+ ServerTestHelper.rewritePathsInGremlinServerSettings(overriddenSettings);
if (GREMLIN_SERVER_EPOLL) {
- overridenSettings.useEpollEventLoop = true;
+ overriddenSettings.useEpollEventLoop = true;
}
- this.server = new GremlinServer(overridenSettings);
+ this.server = new GremlinServer(overriddenSettings);
server.start().join();
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f592e344/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
----------------------------------------------------------------------
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 1db7a50..9256458 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
@@ -66,6 +66,7 @@ import org.apache.tinkerpop.gremlin.structure.util.empty.EmptyGraph;
import org.apache.tinkerpop.gremlin.util.Log4jRecordingAppender;
import org.apache.tinkerpop.gremlin.util.function.Lambda;
import org.hamcrest.CoreMatchers;
+import org.hamcrest.Matchers;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
@@ -75,6 +76,7 @@ import java.nio.channels.ClosedChannelException;
import java.util.ArrayList;
import java.util.Collections;
import java.util.HashMap;
+import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
@@ -376,6 +378,40 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
}
@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;
+ overriddenSettings.scriptEvaluationTimeout = 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 {
final Cluster cluster = TestClientFactory.open();
final Client client = cluster.connect(name.getMethodName());
@@ -985,6 +1021,21 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
}
@Test
+ public void shouldHavePartialContentWithLongResultsCollection() throws Exception {
+ try (SimpleClient client = TestClientFactory.createWebSocketClient()) {
+ final RequestMessage request = RequestMessage.build(Tokens.OPS_EVAL)
+ .addArg(Tokens.ARGS_GREMLIN, "new String[100]").create();
+ final List<ResponseMessage> responses = client.submit(request);
+ assertThat(responses.size(), Matchers.greaterThan(1));
+ for (Iterator<ResponseMessage> it = responses.iterator(); it.hasNext(); ) {
+ ResponseMessage msg = it.next();
+ ResponseStatusCode expected = it.hasNext() ? ResponseStatusCode.PARTIAL_CONTENT : ResponseStatusCode.SUCCESS;
+ assertEquals(expected, msg.getStatus().getCode());
+ }
+ }
+ }
+
+ @Test
public void shouldFailWithBadScriptEval() throws Exception {
try (SimpleClient client = TestClientFactory.createWebSocketClient()) {
final RequestMessage request = RequestMessage.build(Tokens.OPS_EVAL)
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f592e344/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
new file mode 100644
index 0000000..bea318b
--- /dev/null
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
@@ -0,0 +1,110 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.server;
+
+import io.netty.channel.ChannelHandlerContext;
+import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode;
+import org.hamcrest.CoreMatchers;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.mockito.Mockito;
+
+import java.util.Arrays;
+import java.util.UUID;
+import java.util.function.BiFunction;
+
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.fail;
+
+@RunWith(Parameterized.class)
+public class ResponseHandlerContextTest {
+
+ @Parameterized.Parameter(value = 0)
+ public BiFunction<ResponseHandlerContext, ResponseStatusCode, Void> writeInvoker;
+
+ private final ChannelHandlerContext ctx = Mockito.mock(ChannelHandlerContext.class);
+ private final RequestMessage request = RequestMessage.build("test").create();
+ private final Context context = new Context(request, ctx, null, null, null, null);
+ private final ResponseHandlerContext rhc = new ResponseHandlerContext(context);
+
+ @Parameterized.Parameters(name = "{0}")
+ public static Iterable<Object[]> data() {
+ return Arrays.asList(new Object[][] {
+ {
+ new BiFunction<ResponseHandlerContext, ResponseStatusCode, Void>() {
+ @Override
+ public Void apply(ResponseHandlerContext context, ResponseStatusCode code) {
+ context.writeAndFlush(code, "testMessage");
+ return null;
+ }
+
+ @Override
+ public String toString() {
+ return "writeAndFlush(ResponseStatusCode, Object)";
+ }
+ }
+ }, {
+ new BiFunction<ResponseHandlerContext, ResponseStatusCode, Void>() {
+ @Override
+ public Void apply(ResponseHandlerContext context, ResponseStatusCode code) {
+ context.writeAndFlush(ResponseMessage.build(UUID.randomUUID()).code(code).create());
+ return null;
+ }
+
+ @Override
+ public String toString() {
+ return "writeAndFlush(ResponseMessage)";
+ }
+ }
+ },
+ });
+ }
+
+ @Test
+ public void shouldAllowMultipleNonFinalResponses() {
+ writeInvoker.apply(rhc, ResponseStatusCode.AUTHENTICATE);
+ Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(Mockito.any());
+
+ writeInvoker.apply(rhc, ResponseStatusCode.PARTIAL_CONTENT);
+ Mockito.verify(ctx, Mockito.times(2)).writeAndFlush(Mockito.any());
+
+ writeInvoker.apply(rhc, ResponseStatusCode.PARTIAL_CONTENT);
+ Mockito.verify(ctx, Mockito.times(3)).writeAndFlush(Mockito.any());
+ }
+
+ @Test
+ public void shouldAllowAtMostOneFinalResponse() {
+ writeInvoker.apply(rhc, ResponseStatusCode.AUTHENTICATE);
+ Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(Mockito.any());
+
+ writeInvoker.apply(rhc, ResponseStatusCode.SUCCESS);
+ Mockito.verify(ctx, Mockito.times(2)).writeAndFlush(Mockito.any());
+
+ try {
+ writeInvoker.apply(rhc, ResponseStatusCode.SERVER_ERROR_TIMEOUT);
+ fail("Expected an IllegalStateException");
+ } catch (IllegalStateException ex) {
+ assertThat(ex.toString(), CoreMatchers.containsString(request.getRequestId().toString()));
+ }
+ Mockito.verify(ctx, Mockito.times(2)).writeAndFlush(Mockito.any());
+ }
+}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/f592e344/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java
new file mode 100644
index 0000000..cf42737
--- /dev/null
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java
@@ -0,0 +1,73 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.server.op;
+
+import io.netty.channel.ChannelHandlerContext;
+import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.fail;
+
+public class AbstractOpProcessorTest {
+
+ @Test
+ public void deprecatedMakeFrameMethodShouldRedirectCorrectly() throws Exception {
+ ChannelHandlerContext ctx = Mockito.mock(ChannelHandlerContext.class);
+ RequestMessage request = RequestMessage.build("test").create();
+ ArgumentCaptor<ResponseMessage> responseCaptor = ArgumentCaptor.forClass(ResponseMessage.class);
+
+ try {
+ // Induce a NullPointerException to validate error response message writing
+ //noinspection deprecation
+ AbstractOpProcessor.makeFrame(ctx, request, null, true, null, ResponseStatusCode.PARTIAL_CONTENT);
+ fail("Expected a NullPointerException");
+ } catch (NullPointerException expected) {
+ // nop
+ }
+
+ Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(responseCaptor.capture());
+ assertEquals(ResponseStatusCode.SERVER_ERROR_SERIALIZATION, responseCaptor.getValue().getStatus().getCode());
+ assertEquals(request.getRequestId(), responseCaptor.getValue().getRequestId());
+ }
+
+ @Test
+ public void alternativeMakeFrameMethodShouldRedirectCorrectly() throws Exception {
+ ChannelHandlerContext ctx = Mockito.mock(ChannelHandlerContext.class);
+ RequestMessage request = RequestMessage.build("test").create();
+ ArgumentCaptor<ResponseMessage> responseCaptor = ArgumentCaptor.forClass(ResponseMessage.class);
+
+ try {
+ // Induce a NullPointerException to validate error response message writing
+ AbstractOpProcessor.makeFrame(ctx, request, null, true, null, ResponseStatusCode.PARTIAL_CONTENT, null);
+ fail("Expected a NullPointerException");
+ } catch (NullPointerException expected) {
+ // nop
+ }
+
+ Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(responseCaptor.capture());
+ assertEquals(ResponseStatusCode.SERVER_ERROR_SERIALIZATION, responseCaptor.getValue().getStatus().getCode());
+ assertEquals(request.getRequestId(), responseCaptor.getValue().getRequestId());
+ }
+
+}
\ No newline at end of file
[2/9] tinkerpop git commit: TINKERPOP-2005 Handle evaluation
excetions in AbstractEvalOpProcessor
Posted by sp...@apache.org.
TINKERPOP-2005 Handle evaluation excetions in AbstractEvalOpProcessor
Some script evaluation exceptions in AbstractEvalOpProcessor may occur
after the script has started executing. In this situation it it critical
to prevent potentially writing multiple final (e.g. error vs. success)
responses back to the client.
Exceptions that used to escape from evalOpInternal(...) would be
converted to error response messages by OpExecutorHandler, which could
coincide with a successful response from a quick script.
This change makes AbstractEvalOpProcessor do the error message
writing for the same type of exceptions that are handled by
OpExecutorHandler. However, AbstractEvalOpProcessor makes sure that
at most one final reponse message is sent by using
ResponseHandlerContext.
Also ResponseHandlerContext.writeAndFlush(...) methods will no longer
throw exceptions for attempts to send multiple final messages. This is
again to avoid multiple error response messages sent from
OpExecutorHandler.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/b7a44953
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/b7a44953
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/b7a44953
Branch: refs/heads/tp33
Commit: b7a44953c8ac62f308b78709ab2565a78eddb1af
Parents: f592e34
Author: Dmitri Bourlatchkov <dm...@datastax.com>
Authored: Mon Jul 30 12:38:21 2018 -0400
Committer: Dmitri Bourlatchkov <dm...@datastax.com>
Committed: Mon Jul 30 12:38:21 2018 -0400
----------------------------------------------------------------------
.../gremlin/server/ResponseHandlerContext.java | 19 +++---
.../server/op/AbstractEvalOpProcessor.java | 26 +++++++-
.../gremlin/server/op/AbstractOpProcessor.java | 2 +-
.../server/ResponseHandlerContextTest.java | 45 +++++++++++---
.../server/op/AbstractEvalOpProcessorTest.java | 62 ++++++++++++++++++++
5 files changed, 135 insertions(+), 19 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b7a44953/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
index fff4480..3c8c13c 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
@@ -20,6 +20,8 @@ package org.apache.tinkerpop.gremlin.server;
import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -27,7 +29,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
* A context for asynchronously writing response messages related to a particular request.
* <p>The "write" methods of this class ensure that at most one {@link ResponseStatusCode#isFinalResponse() final}
* response message is written to the underlying channel. Attempts to write more than one final response message will
- * result in an {@link IllegalStateException}.</p>
+ * be ignored with a warning log message.</p>
* <p>Note: an object of this class should be used instead of writing to the channel directly when multiple threads
* are expected to produce final response messages concurrently. Callers must ensure that the same
* {@link ResponseHandlerContext} is used by all threads writing response messages for the same request.</p>
@@ -35,6 +37,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
* @author Dmitri Bourlatchkov
*/
public class ResponseHandlerContext {
+ private static final Logger logger = LoggerFactory.getLogger(ResponseHandlerContext.class);
private final Context context;
private final AtomicBoolean finalResponseWritten = new AtomicBoolean();
@@ -52,7 +55,7 @@ public class ResponseHandlerContext {
* {@link ResponseStatusCode#isFinalResponse() final} response is written.
* <p>Note: this method should be used instead of writing to the channel directly when multiple threads
* are expected to produce response messages concurrently.</p>
- * <p>Attempts to write more than one final response message will result in an {@link IllegalStateException}.</p>
+ * <p>Attempts to write more than one final response message will be ignored.</p>
* @see #writeAndFlush(ResponseStatusCode, Object)
*/
public void writeAndFlush(ResponseMessage message) {
@@ -65,16 +68,18 @@ public class ResponseHandlerContext {
* <p>The caller must make sure that the provided response status code matches the content of the message.</p>
* <p>Note: this method should be used instead of writing to the channel directly when multiple threads
* are expected to produce response messages concurrently.</p>
- * <p>Attempts to write more than one final response message will result in an {@link IllegalStateException}.</p>
+ * <p>Attempts to write more than one final response message will be ignored.</p>
* @see #writeAndFlush(ResponseMessage)
*/
public void writeAndFlush(ResponseStatusCode code, Object responseMessage) {
final boolean messageIsFinal = code.isFinalResponse();
- if(!finalResponseWritten.compareAndSet(false, messageIsFinal)) {
- final String errorMessage = String.format("Another final response message was already written for request %s", context.getRequestMessage().getRequestId());
- throw new IllegalStateException(errorMessage);
+ if(finalResponseWritten.compareAndSet(false, messageIsFinal)) {
+ context.getChannelHandlerContext().writeAndFlush(responseMessage);
+ } else {
+ final String logMessage = String.format("Another final response message was already written for request %s, ignoring response code: %s",
+ context.getRequestMessage().getRequestId(), code);
+ logger.warn(logMessage);
}
- context.getChannelHandlerContext().writeAndFlush(responseMessage);
}
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b7a44953/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
index 6ff0452..39168c2 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
@@ -223,9 +223,33 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
* script evaluation.
* @param bindingsSupplier A function that returns the {@link Bindings} to provide to the
* {@link GremlinExecutor#eval} method.
+ * @see #evalOpInternal(ResponseHandlerContext, Supplier, BindingSupplier)
*/
protected void evalOpInternal(final Context context, final Supplier<GremlinExecutor> gremlinExecutorSupplier,
final BindingSupplier bindingsSupplier) throws OpProcessorException {
+ ResponseHandlerContext rhc = new ResponseHandlerContext(context);
+ try {
+ evalOpInternal(rhc, gremlinExecutorSupplier, bindingsSupplier);
+ } catch (Exception ex) {
+ // Exceptions may occur on after the script started executing, therefore corresponding errors must be
+ // reported via the ResponseHandlerContext.
+ logger.warn("Unable to process script evaluation request: " + ex, ex);
+ rhc.writeAndFlush(ResponseMessage.build(context.getRequestMessage())
+ .code(ResponseStatusCode.SERVER_ERROR)
+ .statusAttributeException(ex)
+ .statusMessage(ex.getMessage()).create());
+ }
+ }
+
+ /**
+ * A variant of {@link #evalOpInternal(Context, Supplier, BindingSupplier)} that is suitable for use in situations
+ * when multiple threads may produce {@link ResponseStatusCode#isFinalResponse() final} response messages
+ * concurrently.
+ * @see #evalOpInternal(Context, Supplier, BindingSupplier)
+ */
+ protected void evalOpInternal(final ResponseHandlerContext rhc, final Supplier<GremlinExecutor> gremlinExecutorSupplier,
+ final BindingSupplier bindingsSupplier) throws OpProcessorException {
+ final Context context = rhc.getContext();
final Timer.Context timerContext = evalOpTimer.time();
final ChannelHandlerContext ctx = context.getChannelHandlerContext();
final RequestMessage msg = context.getRequestMessage();
@@ -246,8 +270,6 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
final long seto = args.containsKey(Tokens.ARGS_SCRIPT_EVAL_TIMEOUT) ?
Long.parseLong(args.get(Tokens.ARGS_SCRIPT_EVAL_TIMEOUT).toString()) : settings.scriptEvaluationTimeout;
- ResponseHandlerContext rhc = new ResponseHandlerContext(context);
-
final GremlinExecutor.LifeCycle lifeCycle = GremlinExecutor.LifeCycle.build()
.scriptEvaluationTimeoutOverride(seto)
.afterFailure((b,t) -> {
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b7a44953/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
index 1263c81..c2b6f1f 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
@@ -80,7 +80,7 @@ public abstract class AbstractOpProcessor implements OpProcessor {
}
/**
- * A variant of {@link #handleIterator(Context, Iterator)} that is suitable for use in situations when mutiple
+ * A variant of {@link #handleIterator(Context, Iterator)} that is suitable for use in situations when multiple
* threads may produce {@link ResponseStatusCode#isFinalResponse() final} response messages concurrently.
* @see #handleIterator(Context, Iterator)
*/
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b7a44953/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
index bea318b..6f15a33 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
@@ -19,10 +19,13 @@
package org.apache.tinkerpop.gremlin.server;
import io.netty.channel.ChannelHandlerContext;
+import org.apache.log4j.Logger;
import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode;
-import org.hamcrest.CoreMatchers;
+import org.apache.tinkerpop.gremlin.util.Log4jRecordingAppender;
+import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
@@ -32,8 +35,7 @@ import java.util.Arrays;
import java.util.UUID;
import java.util.function.BiFunction;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.fail;
+import static org.junit.Assert.assertTrue;
@RunWith(Parameterized.class)
public class ResponseHandlerContextTest {
@@ -45,6 +47,7 @@ public class ResponseHandlerContextTest {
private final RequestMessage request = RequestMessage.build("test").create();
private final Context context = new Context(request, ctx, null, null, null, null);
private final ResponseHandlerContext rhc = new ResponseHandlerContext(context);
+ private final Log4jRecordingAppender recordingAppender = new Log4jRecordingAppender();
@Parameterized.Parameters(name = "{0}")
public static Iterable<Object[]> data() {
@@ -79,6 +82,18 @@ public class ResponseHandlerContextTest {
});
}
+ @Before
+ public void addRecordingAppender() {
+ final Logger rootLogger = Logger.getRootLogger();
+ rootLogger.addAppender(recordingAppender);
+ }
+
+ @After
+ public void removeRecordingAppender() {
+ final Logger rootLogger = Logger.getRootLogger();
+ rootLogger.removeAppender(recordingAppender);
+ }
+
@Test
public void shouldAllowMultipleNonFinalResponses() {
writeInvoker.apply(rhc, ResponseStatusCode.AUTHENTICATE);
@@ -99,12 +114,24 @@ public class ResponseHandlerContextTest {
writeInvoker.apply(rhc, ResponseStatusCode.SUCCESS);
Mockito.verify(ctx, Mockito.times(2)).writeAndFlush(Mockito.any());
- try {
- writeInvoker.apply(rhc, ResponseStatusCode.SERVER_ERROR_TIMEOUT);
- fail("Expected an IllegalStateException");
- } catch (IllegalStateException ex) {
- assertThat(ex.toString(), CoreMatchers.containsString(request.getRequestId().toString()));
- }
+ writeInvoker.apply(rhc, ResponseStatusCode.SERVER_ERROR_TIMEOUT);
+ assertTrue(recordingAppender.logContainsAny(".*" + request.getRequestId() + ".*"));
+ assertTrue(recordingAppender.logContainsAny(".*" + ResponseStatusCode.SERVER_ERROR_TIMEOUT + "$"));
+
+ // ensure there were no other writes to the channel
Mockito.verify(ctx, Mockito.times(2)).writeAndFlush(Mockito.any());
}
+
+ @Test
+ public void shouldNotAllowNonFinalMessagesAfterFinalResponse() {
+ writeInvoker.apply(rhc, ResponseStatusCode.SERVER_ERROR_TIMEOUT);
+ Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(Mockito.any());
+
+ writeInvoker.apply(rhc, ResponseStatusCode.PARTIAL_CONTENT);
+ assertTrue(recordingAppender.logContainsAny(".*" + request.getRequestId() + ".*"));
+ assertTrue(recordingAppender.logContainsAny(".*" + ResponseStatusCode.PARTIAL_CONTENT + "$"));
+
+ // ensure there were no other writes to the channel
+ Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(Mockito.any());
+ }
}
\ No newline at end of file
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/b7a44953/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessorTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessorTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessorTest.java
new file mode 100644
index 0000000..6f25e2e
--- /dev/null
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessorTest.java
@@ -0,0 +1,62 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.tinkerpop.gremlin.server.op;
+
+import io.netty.channel.ChannelHandlerContext;
+import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode;
+import org.apache.tinkerpop.gremlin.groovy.engine.GremlinExecutor;
+import org.apache.tinkerpop.gremlin.server.Context;
+import org.apache.tinkerpop.gremlin.server.Settings;
+import org.apache.tinkerpop.gremlin.server.op.standard.StandardOpProcessor;
+import org.hamcrest.CoreMatchers;
+import org.junit.Test;
+import org.mockito.ArgumentCaptor;
+import org.mockito.Mockito;
+
+import javax.script.SimpleBindings;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertThat;
+import static org.mockito.Matchers.anyString;
+
+public class AbstractEvalOpProcessorTest {
+
+ @Test
+ public void evalOpInternalShouldHandleAllEvaluationExceptions() throws OpProcessorException {
+ AbstractEvalOpProcessor processor = new StandardOpProcessor();
+ RequestMessage request = RequestMessage.build("test").create();
+ Settings settings = new Settings();
+ ChannelHandlerContext ctx = Mockito.mock(ChannelHandlerContext.class);
+ ArgumentCaptor<ResponseMessage> responseCaptor = ArgumentCaptor.forClass(ResponseMessage.class);
+
+ GremlinExecutor gremlinExecutor = Mockito.mock(GremlinExecutor.class);
+ Mockito.when(gremlinExecutor.eval(anyString(), anyString(), Mockito.any(), Mockito.<GremlinExecutor.LifeCycle>any()))
+ .thenThrow(new IllegalStateException("test-exception"));
+
+ Context context = new Context(request, ctx, settings, null, gremlinExecutor, null);
+ processor.evalOpInternal(context, context::getGremlinExecutor, SimpleBindings::new);
+
+ Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(responseCaptor.capture());
+ assertEquals(ResponseStatusCode.SERVER_ERROR, responseCaptor.getValue().getStatus().getCode());
+ assertEquals(request.getRequestId(), responseCaptor.getValue().getRequestId());
+ assertThat(responseCaptor.getValue().getStatus().getMessage(), CoreMatchers.containsString("test-exception"));
+ }
+}
\ No newline at end of file
[5/9] tinkerpop git commit: TINKERPOP-2005 Minor code cleanup
Posted by sp...@apache.org.
TINKERPOP-2005 Minor code cleanup
* Declare variables 'final'
* Remove unused variables
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/0e0d26cd
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/0e0d26cd
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/0e0d26cd
Branch: refs/heads/tp33
Commit: 0e0d26cdafca3c2a3244fe224f3f08391669d470
Parents: 63ae13c
Author: Dmitri Bourlatchkov <dm...@datastax.com>
Authored: Tue Jul 31 10:48:12 2018 -0400
Committer: Dmitri Bourlatchkov <dm...@datastax.com>
Committed: Tue Jul 31 10:48:12 2018 -0400
----------------------------------------------------------------------
.../gremlin/server/ResponseHandlerContext.java | 6 +++---
.../gremlin/server/op/AbstractEvalOpProcessor.java | 3 +--
.../gremlin/server/op/AbstractOpProcessor.java | 4 ++--
.../gremlin/server/GremlinServerIntegrateTest.java | 4 ++--
.../gremlin/server/ResponseHandlerContextTest.java | 4 ++--
.../server/op/AbstractEvalOpProcessorTest.java | 14 +++++++-------
.../gremlin/server/op/AbstractOpProcessorTest.java | 12 ++++++------
7 files changed, 23 insertions(+), 24 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0e0d26cd/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
index 3c8c13c..5cc79b5 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
@@ -42,7 +42,7 @@ public class ResponseHandlerContext {
private final Context context;
private final AtomicBoolean finalResponseWritten = new AtomicBoolean();
- public ResponseHandlerContext(Context context) {
+ public ResponseHandlerContext(final Context context) {
this.context = context;
}
@@ -58,7 +58,7 @@ public class ResponseHandlerContext {
* <p>Attempts to write more than one final response message will be ignored.</p>
* @see #writeAndFlush(ResponseStatusCode, Object)
*/
- public void writeAndFlush(ResponseMessage message) {
+ public void writeAndFlush(final ResponseMessage message) {
writeAndFlush(message.getStatus().getCode(), message);
}
@@ -71,7 +71,7 @@ public class ResponseHandlerContext {
* <p>Attempts to write more than one final response message will be ignored.</p>
* @see #writeAndFlush(ResponseMessage)
*/
- public void writeAndFlush(ResponseStatusCode code, Object responseMessage) {
+ public void writeAndFlush(final ResponseStatusCode code, final Object responseMessage) {
final boolean messageIsFinal = code.isFinalResponse();
if(finalResponseWritten.compareAndSet(false, messageIsFinal)) {
context.getChannelHandlerContext().writeAndFlush(responseMessage);
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0e0d26cd/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
index 39168c2..ca1ee53 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
@@ -227,7 +227,7 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
*/
protected void evalOpInternal(final Context context, final Supplier<GremlinExecutor> gremlinExecutorSupplier,
final BindingSupplier bindingsSupplier) throws OpProcessorException {
- ResponseHandlerContext rhc = new ResponseHandlerContext(context);
+ final ResponseHandlerContext rhc = new ResponseHandlerContext(context);
try {
evalOpInternal(rhc, gremlinExecutorSupplier, bindingsSupplier);
} catch (Exception ex) {
@@ -251,7 +251,6 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
final BindingSupplier bindingsSupplier) throws OpProcessorException {
final Context context = rhc.getContext();
final Timer.Context timerContext = evalOpTimer.time();
- final ChannelHandlerContext ctx = context.getChannelHandlerContext();
final RequestMessage msg = context.getRequestMessage();
final GremlinExecutor gremlinExecutor = gremlinExecutorSupplier.get();
final Settings settings = context.getSettings();
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0e0d26cd/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
index c2b6f1f..38ca3e1 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
@@ -278,8 +278,8 @@ public abstract class AbstractOpProcessor implements OpProcessor {
protected static Frame makeFrame(final ChannelHandlerContext ctx, final RequestMessage msg,
final MessageSerializer serializer, final boolean useBinary, final List<Object> aggregate,
final ResponseStatusCode code, final Map<String,Object> responseMetaData) throws Exception {
- Context context = new Context(msg, ctx, null, null, null, null); // dummy context, good only for writing response messages to the channel
- ResponseHandlerContext rhc = new ResponseHandlerContext(context);
+ final Context context = new Context(msg, ctx, null, null, null, null); // dummy context, good only for writing response messages to the channel
+ final ResponseHandlerContext rhc = new ResponseHandlerContext(context);
return makeFrame(rhc, msg, serializer, useBinary, aggregate, code, responseMetaData);
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0e0d26cd/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
----------------------------------------------------------------------
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 0867fd3..eb5def9 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
@@ -1028,8 +1028,8 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
final List<ResponseMessage> responses = client.submit(request);
assertThat(responses.size(), Matchers.greaterThan(1));
for (Iterator<ResponseMessage> it = responses.iterator(); it.hasNext(); ) {
- ResponseMessage msg = it.next();
- ResponseStatusCode expected = it.hasNext() ? ResponseStatusCode.PARTIAL_CONTENT : ResponseStatusCode.SUCCESS;
+ final ResponseMessage msg = it.next();
+ final ResponseStatusCode expected = it.hasNext() ? ResponseStatusCode.PARTIAL_CONTENT : ResponseStatusCode.SUCCESS;
assertEquals(expected, msg.getStatus().getCode());
}
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0e0d26cd/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
index 6f15a33..13c9992 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
@@ -55,7 +55,7 @@ public class ResponseHandlerContextTest {
{
new BiFunction<ResponseHandlerContext, ResponseStatusCode, Void>() {
@Override
- public Void apply(ResponseHandlerContext context, ResponseStatusCode code) {
+ public Void apply(final ResponseHandlerContext context, final ResponseStatusCode code) {
context.writeAndFlush(code, "testMessage");
return null;
}
@@ -68,7 +68,7 @@ public class ResponseHandlerContextTest {
}, {
new BiFunction<ResponseHandlerContext, ResponseStatusCode, Void>() {
@Override
- public Void apply(ResponseHandlerContext context, ResponseStatusCode code) {
+ public Void apply(final ResponseHandlerContext context, final ResponseStatusCode code) {
context.writeAndFlush(ResponseMessage.build(UUID.randomUUID()).code(code).create());
return null;
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0e0d26cd/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessorTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessorTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessorTest.java
index 6f25e2e..72b9c5c 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessorTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessorTest.java
@@ -41,17 +41,17 @@ public class AbstractEvalOpProcessorTest {
@Test
public void evalOpInternalShouldHandleAllEvaluationExceptions() throws OpProcessorException {
- AbstractEvalOpProcessor processor = new StandardOpProcessor();
- RequestMessage request = RequestMessage.build("test").create();
- Settings settings = new Settings();
- ChannelHandlerContext ctx = Mockito.mock(ChannelHandlerContext.class);
- ArgumentCaptor<ResponseMessage> responseCaptor = ArgumentCaptor.forClass(ResponseMessage.class);
+ final AbstractEvalOpProcessor processor = new StandardOpProcessor();
+ final RequestMessage request = RequestMessage.build("test").create();
+ final Settings settings = new Settings();
+ final ChannelHandlerContext ctx = Mockito.mock(ChannelHandlerContext.class);
+ final ArgumentCaptor<ResponseMessage> responseCaptor = ArgumentCaptor.forClass(ResponseMessage.class);
- GremlinExecutor gremlinExecutor = Mockito.mock(GremlinExecutor.class);
+ final GremlinExecutor gremlinExecutor = Mockito.mock(GremlinExecutor.class);
Mockito.when(gremlinExecutor.eval(anyString(), anyString(), Mockito.any(), Mockito.<GremlinExecutor.LifeCycle>any()))
.thenThrow(new IllegalStateException("test-exception"));
- Context context = new Context(request, ctx, settings, null, gremlinExecutor, null);
+ final Context context = new Context(request, ctx, settings, null, gremlinExecutor, null);
processor.evalOpInternal(context, context::getGremlinExecutor, SimpleBindings::new);
Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(responseCaptor.capture());
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/0e0d26cd/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java
index cf42737..a7cee1a 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java
@@ -33,9 +33,9 @@ public class AbstractOpProcessorTest {
@Test
public void deprecatedMakeFrameMethodShouldRedirectCorrectly() throws Exception {
- ChannelHandlerContext ctx = Mockito.mock(ChannelHandlerContext.class);
- RequestMessage request = RequestMessage.build("test").create();
- ArgumentCaptor<ResponseMessage> responseCaptor = ArgumentCaptor.forClass(ResponseMessage.class);
+ final ChannelHandlerContext ctx = Mockito.mock(ChannelHandlerContext.class);
+ final RequestMessage request = RequestMessage.build("test").create();
+ final ArgumentCaptor<ResponseMessage> responseCaptor = ArgumentCaptor.forClass(ResponseMessage.class);
try {
// Induce a NullPointerException to validate error response message writing
@@ -53,9 +53,9 @@ public class AbstractOpProcessorTest {
@Test
public void alternativeMakeFrameMethodShouldRedirectCorrectly() throws Exception {
- ChannelHandlerContext ctx = Mockito.mock(ChannelHandlerContext.class);
- RequestMessage request = RequestMessage.build("test").create();
- ArgumentCaptor<ResponseMessage> responseCaptor = ArgumentCaptor.forClass(ResponseMessage.class);
+ final ChannelHandlerContext ctx = Mockito.mock(ChannelHandlerContext.class);
+ final RequestMessage request = RequestMessage.build("test").create();
+ final ArgumentCaptor<ResponseMessage> responseCaptor = ArgumentCaptor.forClass(ResponseMessage.class);
try {
// Induce a NullPointerException to validate error response message writing
[3/9] tinkerpop git commit: TINKERPOP-2005 Encapsulate test eval
timeout overrides in a method
Posted by sp...@apache.org.
TINKERPOP-2005 Encapsulate test eval timeout overrides in a method
This is to avoid directly exposing other settings whose changes may
not be effective until the server is restarted.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/4ff4b88a
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/4ff4b88a
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/4ff4b88a
Branch: refs/heads/tp33
Commit: 4ff4b88a5b663f86917299691edc6f84a6bd1c67
Parents: b7a4495
Author: Dmitri Bourlatchkov <dm...@datastax.com>
Authored: Mon Jul 30 15:57:14 2018 -0400
Committer: Dmitri Bourlatchkov <dm...@datastax.com>
Committed: Mon Jul 30 15:57:14 2018 -0400
----------------------------------------------------------------------
.../server/AbstractGremlinServerIntegrationTest.java | 13 ++++++++++++-
.../gremlin/server/GremlinServerIntegrateTest.java | 2 +-
2 files changed, 13 insertions(+), 2 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4ff4b88a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
index a8a8853..f11a045 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/AbstractGremlinServerIntegrationTest.java
@@ -39,7 +39,7 @@ import static org.junit.Assume.assumeThat;
*/
public abstract class AbstractGremlinServerIntegrationTest {
protected GremlinServer server;
- protected Settings overriddenSettings;
+ private Settings overriddenSettings;
private final static String epollOption = "gremlin.server.epoll";
private static final boolean GREMLIN_SERVER_EPOLL = "true".equalsIgnoreCase(System.getProperty(epollOption));
private static final Logger logger = LoggerFactory.getLogger(AbstractGremlinServerIntegrationTest.class);
@@ -51,6 +51,17 @@ public abstract class AbstractGremlinServerIntegrationTest {
return settings;
}
+ /**
+ * This method may be called after {@link #startServer()} to (re-)set the script evaluation timeout in
+ * the running server.
+ * @param timeoutInMillis new script evaluation timeout
+ */
+ protected void overrideScriptEvaluationTimeout(final long timeoutInMillis) {
+ // Note: overriding settings in a running server is not guaranteed to work for all settings.
+ // It works for the evaluation timeout, though, because GremlinExecutor is re-created for each evaluation.
+ overriddenSettings.scriptEvaluationTimeout = timeoutInMillis;
+ }
+
public InputStream getSettingsInputStream() {
return AbstractGremlinServerIntegrationTest.class.getResourceAsStream("gremlin-server-integration.yaml");
}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/4ff4b88a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
----------------------------------------------------------------------
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 9256458..0867fd3 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
@@ -394,7 +394,7 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
// 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;
- overriddenSettings.scriptEvaluationTimeout = timeout;
+ overrideScriptEvaluationTimeout(timeout);
try {
client.submit("x = 1 + 1").all().get().get(0).getInt();
[6/9] tinkerpop git commit: TINKERPOP-2005 Set/restore log level in
ResponseHandlerContextTest
Posted by sp...@apache.org.
TINKERPOP-2005 Set/restore log level in ResponseHandlerContextTest
This is to allow these tests to capture intended log messages in the
CI environment.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/fe9c26f6
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/fe9c26f6
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/fe9c26f6
Branch: refs/heads/tp33
Commit: fe9c26f656cf79f513af5b1230f498934a796063
Parents: 0e0d26c
Author: Dmitri Bourlatchkov <dm...@datastax.com>
Authored: Tue Jul 31 12:11:34 2018 -0400
Committer: Dmitri Bourlatchkov <dm...@datastax.com>
Committed: Tue Jul 31 12:11:34 2018 -0400
----------------------------------------------------------------------
.../tinkerpop/gremlin/server/ResponseHandlerContextTest.java | 6 ++++++
1 file changed, 6 insertions(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/fe9c26f6/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
index 13c9992..2babee9 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
@@ -19,6 +19,7 @@
package org.apache.tinkerpop.gremlin.server;
import io.netty.channel.ChannelHandlerContext;
+import org.apache.log4j.Level;
import org.apache.log4j.Logger;
import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
@@ -49,6 +50,8 @@ public class ResponseHandlerContextTest {
private final ResponseHandlerContext rhc = new ResponseHandlerContext(context);
private final Log4jRecordingAppender recordingAppender = new Log4jRecordingAppender();
+ private Level originalLogLevel;
+
@Parameterized.Parameters(name = "{0}")
public static Iterable<Object[]> data() {
return Arrays.asList(new Object[][] {
@@ -86,11 +89,14 @@ public class ResponseHandlerContextTest {
public void addRecordingAppender() {
final Logger rootLogger = Logger.getRootLogger();
rootLogger.addAppender(recordingAppender);
+ originalLogLevel = rootLogger.getLevel();
+ rootLogger.setLevel(Level.ALL);
}
@After
public void removeRecordingAppender() {
final Logger rootLogger = Logger.getRootLogger();
+ rootLogger.setLevel(originalLogLevel);
rootLogger.removeAppender(recordingAppender);
}
[7/9] tinkerpop git commit: Merge branch 'pr-899' into tp32
Posted by sp...@apache.org.
Merge branch 'pr-899' into tp32
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/387cf693
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/387cf693
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/387cf693
Branch: refs/heads/tp33
Commit: 387cf69323b1ea0a7194033fe951db8f9e43b799
Parents: 2d315e8 fe9c26f
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Tue Aug 7 10:51:23 2018 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Tue Aug 7 10:51:23 2018 -0400
----------------------------------------------------------------------
.../driver/message/ResponseStatusCode.java | 7 +
.../driver/message/ResponseStatusCodeTest.java | 36 +++++
.../gremlin/server/ResponseHandlerContext.java | 85 +++++++++++
.../server/op/AbstractEvalOpProcessor.java | 40 ++++--
.../gremlin/server/op/AbstractOpProcessor.java | 34 ++++-
.../AbstractGremlinServerIntegrationTest.java | 20 ++-
.../server/GremlinServerIntegrateTest.java | 51 +++++++
.../server/ResponseHandlerContextTest.java | 143 +++++++++++++++++++
.../server/op/AbstractEvalOpProcessorTest.java | 62 ++++++++
.../server/op/AbstractOpProcessorTest.java | 73 ++++++++++
10 files changed, 535 insertions(+), 16 deletions(-)
----------------------------------------------------------------------
[8/9] tinkerpop git commit: Updated changelog CTR
Posted by sp...@apache.org.
Updated changelog CTR
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/37476a2b
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/37476a2b
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/37476a2b
Branch: refs/heads/tp33
Commit: 37476a2b46467c2d704cbef977225d8da3cddd64
Parents: 387cf69
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Tue Aug 7 10:52:29 2018 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Tue Aug 7 10:52:29 2018 -0400
----------------------------------------------------------------------
CHANGELOG.asciidoc | 1 +
1 file changed, 1 insertion(+)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/37476a2b/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 495abd7..f6413e2 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -23,6 +23,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
[[release-3-2-10]]
=== TinkerPop 3.2.10 (Release Date: NOT OFFICIALLY RELEASED YET)
+* Fixed problem with Gremlin Server sometimes returning an additional message after a failure.
* Added an system error code for failed plugin installs for Gremlin Server `-i` option.
* Match numbers in `choose()` options using `NumberHelper` (match values, ignore data type).
* Added support for GraphSON serialization of `Date` in Javascript.
[9/9] tinkerpop git commit: Merge branch 'tp32' into tp33
Posted by sp...@apache.org.
Merge branch 'tp32' into tp33
Conflicts:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/fa7a7f61
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/fa7a7f61
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/fa7a7f61
Branch: refs/heads/tp33
Commit: fa7a7f61308acc675e4a4c4a6dc863cf9818c3f9
Parents: 2cf551f 37476a2
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Tue Aug 7 12:08:39 2018 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Tue Aug 7 12:08:39 2018 -0400
----------------------------------------------------------------------
CHANGELOG.asciidoc | 1 +
.../driver/message/ResponseStatusCode.java | 7 +
.../driver/message/ResponseStatusCodeTest.java | 36 +++++
.../gremlin/server/ResponseHandlerContext.java | 85 +++++++++++
.../server/op/AbstractEvalOpProcessor.java | 38 ++++-
.../gremlin/server/op/AbstractOpProcessor.java | 34 ++++-
.../AbstractGremlinServerIntegrationTest.java | 20 ++-
.../server/GremlinServerIntegrateTest.java | 51 +++++++
.../server/ResponseHandlerContextTest.java | 143 +++++++++++++++++++
.../server/op/AbstractEvalOpProcessorTest.java | 62 ++++++++
.../server/op/AbstractOpProcessorTest.java | 73 ++++++++++
11 files changed, 535 insertions(+), 15 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/fa7a7f61/CHANGELOG.asciidoc
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/fa7a7f61/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
----------------------------------------------------------------------
diff --cc gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
index 74e9478,ca1ee53..dbf7a44
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractEvalOpProcessor.java
@@@ -242,14 -287,9 +266,14 @@@ public abstract class AbstractEvalOpPro
final Iterator itty = IteratorUtils.asIterator(o);
logger.debug("Preparing to iterate results from - {} - in thread [{}]", msg, Thread.currentThread().getName());
+ if (settings.authentication.enableAuditLog) {
+ String address = context.getChannelHandlerContext().channel().remoteAddress().toString();
+ if (address.startsWith("/") && address.length() > 1) address = address.substring(1);
+ auditLogger.info("User with address {} requested: {}", address, script);
+ }
try {
- handleIterator(context, itty);
+ handleIterator(rhc, itty);
} catch (Exception ex) {
if (managedTransactionsForRequest) attemptRollback(msg, context.getGraphManager(), settings.strictTransactionManagement);
@@@ -271,9 -311,16 +295,9 @@@
// occurs when the TimedInterruptCustomizerProvider is in play
final String errorMessage = String.format("A timeout occurred within the script during evaluation of [%s] - consider increasing the limit given to TimedInterruptCustomizerProvider", msg);
logger.warn(errorMessage);
- ctx.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
+ rhc.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
.statusMessage("Timeout during script evaluation triggered by TimedInterruptCustomizerProvider")
.statusAttributeException(t).create());
- } else if (t instanceof org.apache.tinkerpop.gremlin.groovy.jsr223.TimedInterruptTimeoutException) {
- // occurs when the TimedInterruptCustomizerProvider is in play
- final String errorMessage = String.format("A timeout occurred within the script during evaluation of [%s] - consider increasing the limit given to TimedInterruptCustomizerProvider", msg);
- logger.warn(errorMessage);
- rhc.writeAndFlush(ResponseMessage.build(msg).code(ResponseStatusCode.SERVER_ERROR_TIMEOUT)
- .statusMessage("Timeout during script evaluation triggered by TimedInterruptCustomizerProvider")
- .statusAttributeException(t).create());
} else if (t instanceof TimeoutException) {
final String errorMessage = String.format("Script evaluation exceeded the configured threshold for request [%s]", msg);
logger.warn(errorMessage, t);
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/fa7a7f61/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
----------------------------------------------------------------------
diff --cc gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
index a1689e9,eb5def9..67ad021
--- 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
@@@ -71,7 -74,9 +72,8 @@@ import org.junit.Test
import java.lang.reflect.Field;
import java.nio.channels.ClosedChannelException;
import java.util.ArrayList;
-import java.util.Collections;
import java.util.HashMap;
+ import java.util.Iterator;
import java.util.List;
import java.util.Map;
import java.util.Set;
[4/9] tinkerpop git commit: TINKERPOP-2005 Expand .* imports in
ResponseStatusCodeTest
Posted by sp...@apache.org.
TINKERPOP-2005 Expand .* imports in ResponseStatusCodeTest
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/63ae13c2
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/63ae13c2
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/63ae13c2
Branch: refs/heads/tp33
Commit: 63ae13c24a86f59f84a9b30189bb9a73e22fe50a
Parents: 4ff4b88
Author: Dmitri Bourlatchkov <dm...@datastax.com>
Authored: Tue Jul 31 10:25:01 2018 -0400
Committer: Dmitri Bourlatchkov <dm...@datastax.com>
Committed: Tue Jul 31 10:25:01 2018 -0400
----------------------------------------------------------------------
.../tinkerpop/gremlin/driver/message/ResponseStatusCodeTest.java | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/63ae13c2/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCodeTest.java
----------------------------------------------------------------------
diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCodeTest.java b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCodeTest.java
index a231489..3fc8a78 100644
--- a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCodeTest.java
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/message/ResponseStatusCodeTest.java
@@ -21,7 +21,8 @@ package org.apache.tinkerpop.gremlin.driver.message;
import org.junit.Test;
-import static org.junit.Assert.*;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertTrue;
public class ResponseStatusCodeTest {