Merge branch 'tp33' Conflicts: gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/9357d6a1 Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/9357d6a1 Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/9357d6a1 Branch: refs/heads/master Commit: 9357d6a1fd91cac4bd7149b94ed757399aa810a4 Parents: 1e8baec fa7a7f6 Author: Stephen Mallette <sp...@genoprime.com> Authored: Tue Aug 7 12:54:18 2018 -0400 Committer: Stephen Mallette <sp...@genoprime.com> Committed: Tue Aug 7 12:54:18 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 | 53 +++++++ 11 files changed, 515 insertions(+), 15 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9357d6a1/CHANGELOG.asciidoc ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9357d6a1/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java ---------------------------------------------------------------------- diff --cc gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessor.java index bb368c5,38ca3e1..331b762 --- 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 @@@ -66,8 -72,20 +67,20 @@@ public abstract class AbstractOpProcess * * @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 { + protected void handleIterator(final Context context, final Iterator itty) throws InterruptedException { + handleIterator(new ResponseHandlerContext(context), itty); + } + + /** + * 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) + */ - protected void handleIterator(final ResponseHandlerContext rhc, final Iterator itty) throws TimeoutException, InterruptedException { ++ protected void handleIterator(final ResponseHandlerContext rhc, final Iterator itty) throws InterruptedException { + final Context context = rhc.getContext(); final ChannelHandlerContext ctx = context.getChannelHandlerContext(); final RequestMessage msg = context.getRequestMessage(); final Settings settings = context.getSettings(); @@@ -228,9 -261,32 +241,22 @@@ return Collections.emptyMap(); } + /** - * @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 { + final MessageSerializer serializer, final boolean useBinary, final List<Object> aggregate, + final ResponseStatusCode code, final Map<String,Object> responseMetaData) throws Exception { + 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); + } + + 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 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) http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9357d6a1/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 08b8526,67ad021..f97fb1f --- 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 @@@ -69,7 -71,9 +70,8 @@@ import org.junit.Test import java.lang.reflect.Field; import java.nio.channels.ClosedChannelException; -import java.util.ArrayList; import java.util.HashMap; + import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Set; http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/9357d6a1/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java ---------------------------------------------------------------------- diff --cc gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/op/AbstractOpProcessorTest.java index 0000000,a7cee1a..aba1603 mode 000000,100644..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 @@@ -1,0 -1,73 +1,53 @@@ + /* + * 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 { - 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 - //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 { + 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 + 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()); + } + + }