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:27 UTC

[tinkerpop] branch TINKERPOP-2273 created (now 0ab773a)

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

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


      at 0ab773a  TINKERPOP-2273 Removed ResponseHandlerContext infrastructure

This branch includes the following new commits:

     new 0ab773a  TINKERPOP-2273 Removed ResponseHandlerContext infrastructure

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



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

Posted by sp...@apache.org.
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