[ 
https://issues.apache.org/jira/browse/TINKERPOP-2819?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17649997#comment-17649997
 ] 

ASF GitHub Bot commented on TINKERPOP-2819:
-------------------------------------------

Cole-Greer commented on code in PR #1915:
URL: https://github.com/apache/tinkerpop/pull/1915#discussion_r1053766715


##########
gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Driver/GremlinClientBehaviorIntegrationTests.cs:
##########
@@ -0,0 +1,76 @@
+#region License
+
+/*
+ * 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.
+ */
+
+#endregion
+
+using System;
+using System.Linq;
+using System.Threading.Tasks;
+using Gremlin.Net.Driver;
+using Gremlin.Net.Driver.Exceptions;
+using Gremlin.Net.Driver.Messages;
+using Gremlin.Net.IntegrationTest.Util;
+using Gremlin.Net.Structure;
+using Gremlin.Net.Structure.IO.GraphBinary;
+using Xunit;
+
+namespace Gremlin.Net.IntegrationTest.Driver
+{
+    public class GremlinClientBehaviorIntegrationTests
+    {
+        private static readonly string TestHost = 
ConfigProvider.Configuration["GremlinSocketServerIpAddress"]!;
+
+        private static readonly SocketServerSettings Settings =
+            
SocketServerSettings.FromYaml(ConfigProvider.Configuration["GremlinSocketServerConfig"]);
+
+        
+        [Fact]
+        public async Task shouldTryCreateNewConnectionIfClosedByServer()
+        {
+            var sessionId = Guid.NewGuid().ToString();
+            var poolSettings = new ConnectionPoolSettings {PoolSize = 1};
+            
+            GremlinServer gremlinServer = new GremlinServer(TestHost, 
Settings.PORT);
+            GremlinClient gremlinClient = new GremlinClient(gremlinServer, 
messageSerializer: new GraphBinaryMessageSerializer(), connectionPoolSettings: 
poolSettings, sessionId: sessionId);
+
+            Assert.Equal(1, gremlinClient.NrConnections);
+            
+            Task<Vertex?> response1 = 
gremlinClient.SubmitWithSingleResultAsync<Vertex>(RequestMessage.Build("1").OverrideRequestId(Settings.CLOSE_CONNECTION_REQUEST_ID).Create());
+
+            try
+            {
+                await response1;
+            }
+            catch (ConnectionClosedException e)
+            {
+                Assert.Contains("CloseStatus: NormalClosure", e.Message);

Review Comment:
   Thanks this is a cleaner and more robust solution, I will change it.





> Refactor SimpleSocketServer to be accessible to all GLV's
> ---------------------------------------------------------
>
>                 Key: TINKERPOP-2819
>                 URL: https://issues.apache.org/jira/browse/TINKERPOP-2819
>             Project: TinkerPop
>          Issue Type: Improvement
>          Components: driver
>            Reporter: Cole Greer
>            Priority: Major
>
> Currently there is a large gap in the testing capabilities of the java driver 
> compared to the other GLV's. Part of this gap is the java driver has 
> SimpleSocketServer which provides a useful platform to write tests which 
> require specific response behaviour from the server. Having such a tool for 
> all of the GLV's would allow for testing of many more potential failure cases 
> as well as taking a step towards standardizing the testing approach for all 
> GLV's.
> This work can be divided into 2 main parts.
> Part One: Decoupling SimpleSocketServer from the java driver. This is the 
> most disruptive part of the proposed changes. This has already been discussed 
> [here|https://lists.apache.org/thread/vd7w43xjzvc5rr0135gql9mxhdlcltr9] on 
> the dev list but I will summarize. To avoid having all the GLV's depending on 
> the java driver, SimpleSocketServer and it's related classes should be 
> extracted to a new module gremlin-tools/gremlin-socket-server. Unfortunately 
> the socket server still relies on the following classes in gremlin driver:
> tinkerpop.gremlin.driver.message.*
> tinkerpop.gremlin.driver.ser.*
> tinkerpop.gremlin.driver.MessageSerializer
> tinkerpop.gremlin.driver.Tokens
> To avoid a cyclic dependency between gremlin-driver and 
> gremlin-socket-server. these classes should be moved to another new module 
> gremlin-util which will house any classes which are to be shared between the 
> driver and server. Moving these classes to a new module and package will 
> break import lines and will need to be left until 3.7.
> The full list of classes moved into gremlin-util is as follows:
>  
> ||Old Name/Location||New Name/Location||
> |org.apache.tinkerpop.gremlin.driver.MessageSerializer|org.apache.tinkerpop.gremlin.util.MessageSerializer|
> |org.apache.tinkerpop.gremlin.driver.Tokens|org.apache.tinkerpop.gremlin.util.Tokens|
> |org.apache.tinkerpop.gremlin.driver.message.RequestMessage|org.apache.tinkerpop.gremlin.util.message.RequestMessage|
> |org.apache.tinkerpop.gremlin.driver.message.ResponseMessage|org.apache.tinkerpop.gremlin.util.message.ResponseMessage|
> |org.apache.tinkerpop.gremlin.driver.message.ResponseResult|org.apache.tinkerpop.gremlin.util.message.ResponseResult|
> |org.apache.tinkerpop.gremlin.driver.message.ResponseStatus|org.apache.tinkerpop.gremlin.util.message.ResponseStatus|
> |org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode|org.apache.tinkerpop.gremlin.util.message.ResponseStatusCode|
> |org.apache.tinkerpop.gremlin.driver.ser.AbstractGraphSONMessageSerializerV1d0|org.apache.tinkerpop.gremlin.util.ser.AbstractGraphSONMessageSerializerV1d0|
> |org.apache.tinkerpop.gremlin.driver.ser.AbstractGraphSONMessageSerializerV2d0|org.apache.tinkerpop.gremlin.util.ser.AbstractGraphSONMessageSerializerV2d0|
> |org.apache.tinkerpop.gremlin.driver.ser.AbstractMessageSerializer|org.apache.tinkerpop.gremlin.util.ser.AbstractMessageSerializer|
> |org.apache.tinkerpop.gremlin.driver.ser.GraphBinaryMessageSerializerV1|org.apache.tinkerpop.gremlin.util.ser.GraphBinaryMessageSerializerV1|
> |org.apache.tinkerpop.gremlin.driver.ser.GraphSONMessageSerializerGremlinV1d0|org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerGremlinV1d0|
> |org.apache.tinkerpop.gremlin.driver.ser.GraphSONMessageSerializerGremlinV2d0|org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerGremlinV2d0|
> |org.apache.tinkerpop.gremlin.driver.ser.GraphSONMessageSerializerV1d0|org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV1d0|
> |org.apache.tinkerpop.gremlin.driver.ser.GraphSONMessageSerializerV2d0|org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV2d0|
> |org.apache.tinkerpop.gremlin.driver.ser.GraphSONMessageSerializerV3d0|org.apache.tinkerpop.gremlin.util.ser.GraphSONMessageSerializerV3d0|
> |org.apache.tinkerpop.gremlin.driver.ser.MessageTextSerializer|org.apache.tinkerpop.gremlin.util.ser.MessageTextSerializer|
> |org.apache.tinkerpop.gremlin.driver.ser.NettyBuffer|org.apache.tinkerpop.gremlin.util.ser.NettyBuffer|
> |org.apache.tinkerpop.gremlin.driver.ser.NettyBufferFactory|org.apache.tinkerpop.gremlin.util.ser.NettyBufferFactory|
> |org.apache.tinkerpop.gremlin.driver.ser.RequestMessageGryoSerializer|org.apache.tinkerpop.gremlin.util.ser.RequestMessageGryoSerializer|
> |org.apache.tinkerpop.gremlin.driver.ser.ResponseMessageGryoSerializer|org.apache.tinkerpop.gremlin.util.ser.ResponseMessageGryoSerializer|
> |org.apache.tinkerpop.gremlin.driver.ser.SerTokens|org.apache.tinkerpop.gremlin.util.ser.SerTokens|
> |org.apache.tinkerpop.gremlin.driver.ser.SerializationException|org.apache.tinkerpop.gremlin.util.ser.SerializationException|
> |org.apache.tinkerpop.gremlin.driver.ser.Serializers|org.apache.tinkerpop.gremlin.util.ser.Serializers|
> |org.apache.tinkerpop.gremlin.driver.ser.binary.RequestMessageSerializer|org.apache.tinkerpop.gremlin.util.ser.binary.RequestMessageSerializer|
> |org.apache.tinkerpop.gremlin.driver.ser.binary.ResponseMessageSerializer|org.apache.tinkerpop.gremlin.util.ser.binary.ResponseMessageSerializer|
>  
>  
> The second part of this refactor is to reconfigure the newly extracted 
> gremlin-socket-server to be usable by all of the GLV's. This will be done by 
> dockerizing the socket server and have the container run during the testing 
> phase of the GLV's. There is still some consideration to be done as to how 
> the GLV's should best interact with this server. Currently Junit will start 
> and stop the server for each individual test, each test has direct access to 
> the server object and can control it as needed. The GLV's will not have the 
> same direct control over the server. Any control options or behaviour needed 
> will either need to be encoded in the server itself as custom behaviour 
> triggered by specific request ID's or control through some external wrapper 
> or interface around the server. There is still consideration needed as to how 
> this should be done. Any comments on desired functionality or behaviour would 
> be greatly appreciated.
> If at some point in time it is deemed desirable to bring 
> gremlin-socket-server to all GLV's in 3.5.x/3.6.x, a good starting point 
> would be to backport 
> [this|https://github.com/apache/tinkerpop/pull/1850/commits/f5b6abb89e5846768ffe702c1d3842e5d29abed5]
>  commit which creates the new modules and moves all of the classes without 
> changing package names.



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to