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 2019/07/26 14:00:28 UTC

[tinkerpop] 01/01: TINKERPOP-2273 Removed ResponseHandlerContext infrastructure

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

spmallette pushed a commit to branch TINKERPOP-2273
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git

commit 0ab773aee060051bcd3f0e416ce59b740b312322
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Fri Jul 26 09:59:29 2019 -0400

    TINKERPOP-2273 Removed ResponseHandlerContext infrastructure
    
    This class was previously deprecated along 3.3.8 and is no longer necessary.
---
 CHANGELOG.asciidoc                                 |   1 +
 docs/src/upgrade/release-3.5.x.asciidoc            |   8 +
 .../gremlin/server/ResponseHandlerContext.java     |  92 ------------
 .../gremlin/server/op/AbstractEvalOpProcessor.java |  10 --
 .../gremlin/server/op/AbstractOpProcessor.java     |  53 -------
 .../gremlin/server/ResponseHandlerContextTest.java | 161 ---------------------
 6 files changed, 9 insertions(+), 316 deletions(-)

diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 23bd523..f8530d9 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -35,6 +35,7 @@ This release also includes changes from <<release-3-4-3, 3.4.3>>.
 * Removed previously deprecated `Serializers.DEFAULT_RESULT_SERIALIZER` and `Serializers.DEFAULT_REQUEST_SERIALIZER`.
 * Removed previously deprecated `decr` and `incr` from `Order`.
 * Removed previously deprecated `TraversalSource.withRemote()`.
+* Removed previously deprecated `ResponseHandlerContext` infrastructure.
 * Removed previously deprecated `VertexProgram` related infrastructure.
 * Removed previously deprecated SSL settings: `keyCertChainFile`, `keyFile`, `keyPassword` and `trustCertChainFile` and related infrastructure.
 * Removed previously deprecated `PropertyMapStep` constructor and `isIncludeTokens`.
diff --git a/docs/src/upgrade/release-3.5.x.asciidoc b/docs/src/upgrade/release-3.5.x.asciidoc
index fa529ef..1218dec 100644
--- a/docs/src/upgrade/release-3.5.x.asciidoc
+++ b/docs/src/upgrade/release-3.5.x.asciidoc
@@ -142,12 +142,20 @@ The following deprecated classes, methods or fields have been removed in this ve
 ** `org.apache.tinkerpop.gremlin.server.Settings.SslSettings.keyFile`
 ** `org.apache.tinkerpop.gremlin.server.Settings.SslSettings.keyPassword`
 ** `org.apache.tinkerpop.gremlin.server.Settings.SslSettings.trustCertificateChainFile`
+** `org.apache.tinkerpop.gremlin.server.ResponseHandlerContext`
+** `org.apache.tinkerpop.gremlin.server.op.AbstractEvalOpProcessor.evalOpInternal(ResponseHandlerContext, Supplier, BindingSupplier)`
 ** `org.apache.tinkerpop.gremlin.server.op.AbstractOpProcessor.generateMetaData(ChannelHandlerContext, RequestMessage, ResponseStatusCode, Iterator)`
+** `org.apache.tinkerpop.gremlin.server.op.AbstractOpProcessor.handleIterator(ResponseHandlerContext, Iterator)`
+** `org.apache.tinkerpop.gremlin.server.op.AbstractOpProcessor.makeFrame(ChannelHandlerContext, RequestMessage, MessageSerializer, boolean, List, ResponseStatusCode, Map)`
+** `org.apache.tinkerpop.gremlin.server.op.AbstractOpProcessor.makeFrame(Context, RequestMessage, MessageSerializer, boolean, List, ResponseStatusCode, Map)`
+** `org.apache.tinkerpop.gremlin.server.op.AbstractOpProcessor.makeFrame(ResponseHandlerContext, RequestMessage, MessageSerializer, boolean, List, ResponseStatusCode, Map)`
+** `org.apache.tinkerpop.gremlin.server.op.AbstractOpProcessor.makeFrame(ResponseHandlerContext, RequestMessage, MessageSerializer, boolean, List, ResponseStatusCode, Map, Map)`
 
 See: link:https://issues.apache.org/jira/browse/TINKERPOP-2080[TINKERPOP-2080],
 link:https://issues.apache.org/jira/browse/TINKERPOP-2231[TINKERPOP-2231],
 link:https://issues.apache.org/jira/browse/TINKERPOP-2233[TINKERPOP-2233],
 link:https://issues.apache.org/jira/browse/TINKERPOP-2239[TINKERPOP-2239],
+link:https://issues.apache.org/jira/browse/TINKERPOP-2273[TINKERPOP-2273],
 link:http://tinkerpop.apache.org/docs/3.5.0/upgrade/#_ssl_security[3.2.10 Upgrade Documentation for SSL]
 
 === Upgrading for Provider
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
deleted file mode 100644
index d927fd8..0000000
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContext.java
+++ /dev/null
@@ -1,92 +0,0 @@
-/*
- * 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 org.apache.tinkerpop.gremlin.server.handler.Frame;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
-
-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
- * 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>
- *
- * @author Dmitri Bourlatchkov
- * @deprecated As of release 3.3.8, replaced by {@link Context}.
- */
-@Deprecated
-public class ResponseHandlerContext {
-    private static final Logger logger = LoggerFactory.getLogger(ResponseHandlerContext.class);
-
-    private final Context context;
-    private final AtomicBoolean finalResponseWritten = new AtomicBoolean();
-
-    public ResponseHandlerContext(final 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 be ignored.</p>
-     * @see #writeAndFlush(ResponseStatusCode, Object)
-     */
-    public void writeAndFlush(final 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 be ignored.</p>
-     * @see #writeAndFlush(ResponseMessage)
-     */
-    public void writeAndFlush(final ResponseStatusCode code, final Object responseMessage) {
-        final boolean messageIsFinal = code.isFinalResponse();
-        if(finalResponseWritten.compareAndSet(false, messageIsFinal)) {
-            context.getChannelHandlerContext().writeAndFlush(responseMessage);
-        } else {
-            if (responseMessage instanceof Frame) {
-                ((Frame) responseMessage).tryRelease();
-            }
-
-            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);
-        }
-
-    }
-}
\ No newline at end of file
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 798ee6b..d0f9268 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
@@ -30,7 +30,6 @@ import org.apache.tinkerpop.gremlin.process.traversal.Order;
 import org.apache.tinkerpop.gremlin.process.traversal.Pop;
 import org.apache.tinkerpop.gremlin.process.traversal.Scope;
 import org.apache.tinkerpop.gremlin.server.OpProcessor;
-import org.apache.tinkerpop.gremlin.server.ResponseHandlerContext;
 import org.apache.tinkerpop.gremlin.structure.Column;
 import org.apache.tinkerpop.gremlin.structure.T;
 import org.apache.tinkerpop.gremlin.server.Context;
@@ -191,15 +190,6 @@ public abstract class AbstractEvalOpProcessor extends AbstractOpProcessor {
     }
 
     /**
-     * @deprecated As of release 3.3.8, replaced by {@link #evalOpInternal(Context, Supplier, BindingSupplier)}.
-     */
-    @Deprecated
-    protected void evalOpInternal(final ResponseHandlerContext ctx, final Supplier<GremlinExecutor> gremlinExecutorSupplier,
-                                  final BindingSupplier bindingsSupplier) throws OpProcessorException {
-        evalOpInternal(ctx.getContext(), gremlinExecutorSupplier, bindingsSupplier);
-    }
-
-    /**
      * A generalized implementation of the "eval" operation.  It handles script evaluation and iteration of results
      * so as to write {@link ResponseMessage} objects down the Netty pipeline.  It also handles script timeouts,
      * iteration timeouts, metrics and building bindings.  Note that result iteration is delegated to the
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 e869691..433fd83 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
@@ -28,7 +28,6 @@ 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;
@@ -64,14 +63,6 @@ public abstract class AbstractOpProcessor implements OpProcessor {
     }
 
     /**
-     * @deprecated As of release 3.3.8, not replaced.
-     */
-    @Deprecated
-    protected void handleIterator(final ResponseHandlerContext rhc, final Iterator itty) throws InterruptedException {
-        handleIterator(rhc.getContext(), itty);
-    }
-
-    /**
      * Provides a generic way of iterating a result set back to the client.
      *
      * @param context The Gremlin Server {@link Context} object containing settings, request message, etc.
@@ -261,50 +252,6 @@ public abstract class AbstractOpProcessor implements OpProcessor {
         return metaData;
     }
 
-    /**
-     * @deprecated As of release 3.3.8, replaced by {@link #makeFrame(Context, 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, final Map<String,Object> responseMetaData,
-                                     final Map<String,Object> statusAttributes) 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, statusAttributes);
-    }
-
-    /**
-     * @deprecated As of release 3.3.8, replaced by {@link #makeFrame(Context, RequestMessage, MessageSerializer, boolean, List, ResponseStatusCode, Map)}.
-     */
-    @Deprecated
-    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 {
-        return makeFrame(rhc.getContext(), msg, serializer, useBinary,aggregate, code, responseMetaData);
-    }
-
-    /**
-     * @deprecated As of release 3.3.8, replaced by {@link #makeFrame(Context, RequestMessage, MessageSerializer, boolean, List, ResponseStatusCode, Map)}.
-     */
-    @Deprecated
-    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,
-                                     final Map<String,Object> statusAttributes) throws Exception {
-        return makeFrame(rhc.getContext(), msg, serializer, useBinary,aggregate, code, responseMetaData, statusAttributes);
-    }
-
-    /**
-     * @deprecated As of release 3.4.3, replaced by {@link #makeFrame(Context, RequestMessage, MessageSerializer, boolean, List, ResponseStatusCode, Map)}.
-     */
-    @Deprecated
-    protected static Frame makeFrame(final Context ctx, final RequestMessage msg,
-                                     final MessageSerializer serializer, final boolean useBinary, final List<Object> aggregate,
-                                     final ResponseStatusCode code, final Map<String,Object> responseMetaData) throws Exception {
-        return makeFrame(ctx, msg, serializer, useBinary,aggregate, code, responseMetaData, Collections.emptyMap());
-    }
-
     protected static Frame makeFrame(final Context ctx, final RequestMessage msg,
                                      final MessageSerializer serializer, final boolean useBinary, final List<Object> aggregate,
                                      final ResponseStatusCode code, final Map<String,Object> responseMetaData,
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
deleted file mode 100644
index f07d309..0000000
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/ResponseHandlerContextTest.java
+++ /dev/null
@@ -1,161 +0,0 @@
-/*
- * 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.log4j.Level;
-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.apache.tinkerpop.gremlin.server.handler.Frame;
-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;
-import org.mockito.Mockito;
-
-import java.util.Arrays;
-import java.util.UUID;
-import java.util.function.BiFunction;
-
-import static org.junit.Assert.assertTrue;
-
-@RunWith(Parameterized.class)
-public class ResponseHandlerContextTest {
-
-    @Parameterized.Parameter(value = 0)
-    public BiFunction<Context, 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 Log4jRecordingAppender recordingAppender = new Log4jRecordingAppender();
-
-    private Level originalLogLevel;
-
-    @Parameterized.Parameters(name = "{0}")
-    public static Iterable<Object[]> data() {
-        return Arrays.asList(new Object[][] {
-            {
-                new BiFunction<Context, ResponseStatusCode, Void>() {
-                    @Override
-                    public Void apply(final Context context, final ResponseStatusCode code) {
-                        context.writeAndFlush(code, "testMessage");
-                        return null;
-                    }
-
-                    @Override
-                    public String toString() {
-                        return "writeAndFlush(ResponseStatusCode, Object)";
-                    }
-                }
-            }, {
-                new BiFunction<Context, ResponseStatusCode, Void>() {
-                    @Override
-                    public Void apply(final Context context, final ResponseStatusCode code) {
-                        context.writeAndFlush(ResponseMessage.build(UUID.randomUUID()).code(code).create());
-                        return null;
-                    }
-
-                    @Override
-                    public String toString() {
-                        return "writeAndFlush(ResponseMessage)";
-                    }
-                }
-            },
-        });
-    }
-
-    @Before
-    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);
-    }
-
-    @Test
-    public void shouldAllowMultipleNonFinalResponses() {
-        writeInvoker.apply(context, ResponseStatusCode.AUTHENTICATE);
-        Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(Mockito.any());
-
-        writeInvoker.apply(context, ResponseStatusCode.PARTIAL_CONTENT);
-        Mockito.verify(ctx, Mockito.times(2)).writeAndFlush(Mockito.any());
-
-        writeInvoker.apply(context, ResponseStatusCode.PARTIAL_CONTENT);
-        Mockito.verify(ctx, Mockito.times(3)).writeAndFlush(Mockito.any());
-    }
-
-    @Test
-    public void shouldAllowAtMostOneFinalResponse() {
-        writeInvoker.apply(context, ResponseStatusCode.AUTHENTICATE);
-        Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(Mockito.any());
-
-        writeInvoker.apply(context, ResponseStatusCode.SUCCESS);
-        Mockito.verify(ctx, Mockito.times(2)).writeAndFlush(Mockito.any());
-
-        writeInvoker.apply(context, 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(context, ResponseStatusCode.SERVER_ERROR_TIMEOUT);
-        Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(Mockito.any());
-
-        writeInvoker.apply(context, 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());
-    }
-
-    @Test
-    public void shouldReleaseIgnoredFrames() {
-        writeInvoker.apply(context, ResponseStatusCode.SERVER_ERROR_TIMEOUT);
-        Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(Mockito.any());
-
-        Frame frame = Mockito.mock(Frame.class);
-        context.writeAndFlush(ResponseStatusCode.SUCCESS, frame);
-
-        assertTrue(recordingAppender.logContainsAny(".*" + request.getRequestId() + ".*"));
-        assertTrue(recordingAppender.logContainsAny(".*" + ResponseStatusCode.SUCCESS + "$"));
-
-        // ensure there were no other writes to the channel
-        Mockito.verify(ctx, Mockito.times(1)).writeAndFlush(Mockito.any());
-
-        // ensure the frame was released
-        Mockito.verify(frame, Mockito.times(1)).tryRelease();
-    }
-}
\ No newline at end of file