You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by co...@apache.org on 2024/10/16 23:53:53 UTC

(tinkerpop) branch 3.7-dev updated: [TINKERPOP 2700] Make WS Compression Configurable in Remaining GLVs 3.7 (#2833)

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

colegreer pushed a commit to branch 3.7-dev
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


The following commit(s) were added to refs/heads/3.7-dev by this push:
     new 944eae0c25 [TINKERPOP 2700] Make WS Compression Configurable in Remaining GLVs 3.7 (#2833)
944eae0c25 is described below

commit 944eae0c25e21acca3aa06c6c909bc3ca517858d
Author: Cole Greer <11...@users.noreply.github.com>
AuthorDate: Wed Oct 16 16:53:48 2024 -0700

    [TINKERPOP 2700] Make WS Compression Configurable in Remaining GLVs 3.7 (#2833)
    
    Adds new enableCompression configuration to toggle use of permessage deflate
    compression in Java, Python, and JS. This was brings these GLVs inline with
    .Net and Go which were previously configurable.
    
    This change does not modify the default behaviour regarding compression in any
    of the GLVs.
---
 .../WebSocketClientBehaviorIntegrateTest.java      | 57 ++++++++++++++++++++++
 .../javascript/gremlin-javascript/test/helper.js   |  7 +++
 .../test/integration/client-behavior-tests.js      | 27 ++++++++++
 gremlin-python/src/main/python/tests/conftest.py   |  4 +-
 .../driver/test_web_socket_client_behavior.py      | 32 ++++++++++++
 .../conf/test-ws-gremlin.yaml                      |  4 ++
 .../socket/server/SocketServerSettings.java        |  6 +++
 .../socket/server/TestWSGremlinInitializer.java    |  6 +++
 8 files changed, 142 insertions(+), 1 deletion(-)

diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/WebSocketClientBehaviorIntegrateTest.java b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/WebSocketClientBehaviorIntegrateTest.java
index 1ff581416c..5cd461a91f 100644
--- a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/WebSocketClientBehaviorIntegrateTest.java
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/WebSocketClientBehaviorIntegrateTest.java
@@ -53,6 +53,7 @@ import java.util.concurrent.atomic.AtomicBoolean;
 import static org.hamcrest.core.Is.is;
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
 
@@ -142,6 +143,62 @@ public class WebSocketClientBehaviorIntegrateTest {
         assertEquals("", returnedUserAgent);
     }
 
+    /**
+     * Tests that client is correctly requesting permessage deflate compression is used
+     */
+    @Test
+    public void shouldRequestCompressionByDefault() {
+        final Cluster cluster = Cluster.build("localhost").port(settings.PORT)
+                .minConnectionPoolSize(1)
+                .maxConnectionPoolSize(1)
+                .serializer(Serializers.GRAPHSON_V2)
+                .create();
+        final Client.ClusteredClient client = cluster.connect();
+
+        // trigger the testing server to return captured sec-websocket-extensions header
+        String returnedWsExtensions = client.submit("1", RequestOptions.build()
+                .overrideRequestId(settings.SEC_WEBSOCKET_EXTENSIONS).create()).one().getString();
+        assertTrue(returnedWsExtensions.contains("permessage-deflate;"));
+    }
+
+    /**
+     * Tests that client is correctly requesting permessage deflate compression is used
+     */
+    @Test
+    public void shouldRequestCompressionWhenEnabled() {
+        final Cluster cluster = Cluster.build("localhost").port(settings.PORT)
+                .minConnectionPoolSize(1)
+                .maxConnectionPoolSize(1)
+                .serializer(Serializers.GRAPHSON_V2)
+                .enableCompression(true)
+                .create();
+        final Client.ClusteredClient client = cluster.connect();
+
+        // trigger the testing server to return captured sec-websocket-extensions header
+        String returnedWsExtensions = client.submit("1", RequestOptions.build()
+                .overrideRequestId(settings.SEC_WEBSOCKET_EXTENSIONS).create()).one().getString();
+        assertTrue(returnedWsExtensions.contains("permessage-deflate;"));
+    }
+
+    /**
+     * Tests that no user agent is sent to server when that behaviour is disabled.
+     */
+    @Test
+    public void shouldNotRequestCompressionWhenDisabled() {
+        final Cluster cluster = Cluster.build("localhost").port(settings.PORT)
+                .minConnectionPoolSize(1)
+                .maxConnectionPoolSize(1)
+                .serializer(Serializers.GRAPHSON_V2)
+                .enableCompression(false)
+                .create();
+        final Client.ClusteredClient client = cluster.connect();
+
+        // trigger the testing server to return captured sec-websocket-extensions header
+        String returnedWsExtensions = client.submit("1", RequestOptions.build()
+                .overrideRequestId(settings.SEC_WEBSOCKET_EXTENSIONS).create()).one().getString();
+        assertFalse(returnedWsExtensions.contains("permessage-deflate;"));
+    }
+
     /**
      * Constructs a deadlock situation when initializing a {@link Client} object in sessionless form that leads to
      * hanging behavior in low resource environments (TINKERPOP-2504) and for certain configurations of the
diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/helper.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/helper.js
index 6232570f68..9421853c33 100644
--- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/helper.js
+++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/helper.js
@@ -102,6 +102,13 @@ exports.getGremlinSocketServerClient = function getGremlinSocketServerClient(tra
   return new Client(url, { traversalSource, mimeType });
 };
 
+exports.getGremlinSocketServerClientWithOptions = function getGremlinSocketServerClient(traversalSource, options) {
+  const settings = exports.getGremlinSocketServerSettings();
+  const url = socketServerUrl + settings.PORT + '/gremlin';
+  const mimeType = getMimeTypeFromSocketServerSettings(settings)
+  return new Client(url, { traversalSource, mimeType, ...options});
+};
+
 exports.getGremlinSocketServerClientNoUserAgent = function getGremlinSocketServerClient(traversalSource) {
   const settings = exports.getGremlinSocketServerSettings();
   const url = socketServerUrl + settings.PORT + '/gremlin';
diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/client-behavior-tests.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/client-behavior-tests.js
index 20f5362ccf..b98ae3e86f 100644
--- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/client-behavior-tests.js
+++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/client-behavior-tests.js
@@ -63,6 +63,33 @@ describe('Client', function () {
 
             await noUserAgentClient.close();
         });
+        it('should not request permessage deflate compression by default', async function () {
+            const result = await client.submit('1', null, {requestId: settings.SEC_WEBSOCKET_EXTENSIONS});
+            const returnedExtensions = result.first()
+            assert.ok(returnedExtensions == undefined || !returnedExtensions.includes("permessage-deflate;"))
+        });
+        it('should not request permessage deflate compression when disabled', async function () {
+            const noCompressionClient = helper.getGremlinSocketServerClientWithOptions('gmodern',
+                {enableCompression: false});
+            const result = await noCompressionClient.submit('1', null,
+                {requestId: settings.SEC_WEBSOCKET_EXTENSIONS});
+
+            const returnedExtensions = result.first()
+            assert.ok(returnedExtensions == undefined || !returnedExtensions.includes("permessage-deflate;"))
+
+            await noCompressionClient.close();
+        });
+        it('should request permessage deflate compression when enabled', async function () {
+            const compressionClient = helper.getGremlinSocketServerClientWithOptions('gmodern',
+                {enableCompression: true});
+            const result = await compressionClient.submit('1', null,
+                {requestId: settings.SEC_WEBSOCKET_EXTENSIONS});
+
+            const returnedExtensions = result.first()
+            assert.ok(returnedExtensions.includes("permessage-deflate;"))
+
+            await compressionClient.close();
+        });
         it('should send per request settings to server', async function () {
             const resultSet = await client.submit('1', null, {
                 requestId: settings.PER_REQUEST_SETTINGS_REQUEST_ID,
diff --git a/gremlin-python/src/main/python/tests/conftest.py b/gremlin-python/src/main/python/tests/conftest.py
index a48fb5fa27..4739cd6f27 100644
--- a/gremlin-python/src/main/python/tests/conftest.py
+++ b/gremlin-python/src/main/python/tests/conftest.py
@@ -110,9 +110,11 @@ def gremlin_socket_server_serializer(socket_server_settings):
 
 @pytest.fixture
 def socket_server_client(request, socket_server_settings, gremlin_socket_server_serializer):
+    marker = request.node.get_closest_marker("client_kwargs")
+    client_kwargs = marker.args[0] if marker is not None else dict()
     url = gremlin_socket_server_url.format(socket_server_settings["PORT"])
     try:
-        client = Client(url, 'g', pool_size=1, message_serializer=gremlin_socket_server_serializer)
+        client = Client(url, 'g', pool_size=1, message_serializer=gremlin_socket_server_serializer, **client_kwargs)
     except OSError:
         pytest.skip('Gremlin Socket Server is not running')
     else:
diff --git a/gremlin-python/src/main/python/tests/driver/test_web_socket_client_behavior.py b/gremlin-python/src/main/python/tests/driver/test_web_socket_client_behavior.py
index 66848bdaaa..8c5ff31a27 100644
--- a/gremlin-python/src/main/python/tests/driver/test_web_socket_client_behavior.py
+++ b/gremlin-python/src/main/python/tests/driver/test_web_socket_client_behavior.py
@@ -23,6 +23,11 @@ __author__ = 'Cole Greer (cole@colegreer.ca)'
 import re
 import operator
 from functools import reduce
+
+import pytest
+
+from tests import conftest
+
 from gremlin_python.driver import useragent
 
 
@@ -69,6 +74,33 @@ def test_should_not_include_user_agent_in_handshake_request_if_disabled(socket_s
     # during connection requests.
     assert re.search("^Python/(\d\.)*\d aiohttp/(\d\.)*\d", user_agent_response)
 
+
+# Tests that client does not request permessage deflate compression by default
+def test_should_not_request_compression_by_default(socket_server_client, socket_server_settings):
+    response = socket_server_client.submit(
+        "1", request_options={'requestId': socket_server_settings["SEC_WEBSOCKET_EXTENSIONS"]}).one()[0]
+
+    assert 'permessage-deflate;' not in response
+
+
+# Tests that client does not request permessage deflate compression when disabled
+@pytest.mark.client_kwargs({'enable_compression': False})
+def test_should_not_request_compression_when_disabled(socket_server_client, socket_server_settings):
+    response = socket_server_client.submit(
+        "1", request_options={'requestId': socket_server_settings["SEC_WEBSOCKET_EXTENSIONS"]}).one()[0]
+
+    assert 'permessage-deflate;' not in response
+
+
+# Tests that client requests permessage deflate compression when enabled
+@pytest.mark.client_kwargs({'enable_compression': True})
+def test_should_request_compression_when_enabled(socket_server_client, socket_server_settings,):
+    response = socket_server_client.submit(
+        "1", request_options={'requestId': socket_server_settings["SEC_WEBSOCKET_EXTENSIONS"]}).one()[0]
+
+    assert 'permessage-deflate;' in response
+
+
 # Tests that client is correctly sending all overridable per request settings (requestId, batchSize,
 # evaluationTimeout, and userAgent) to the server.
 def test_should_send_per_request_settings_to_server(socket_server_client, socket_server_settings):
diff --git a/gremlin-tools/gremlin-socket-server/conf/test-ws-gremlin.yaml b/gremlin-tools/gremlin-socket-server/conf/test-ws-gremlin.yaml
index a358d4ce12..560e40eff5 100644
--- a/gremlin-tools/gremlin-socket-server/conf/test-ws-gremlin.yaml
+++ b/gremlin-tools/gremlin-socket-server/conf/test-ws-gremlin.yaml
@@ -50,6 +50,10 @@ CLOSE_CONNECTION_REQUEST_ID_2: 3c4cf18a-c7f2-4dad-b9bf-5c701eb33000
 # that was captured during the web socket handshake.
 USER_AGENT_REQUEST_ID: 20ad7bfb-4abf-d7f4-f9d3-9f1d55bee4ad
 
+# If a request with this ID comes to the server, the server responds with the sec-websocket-extensions header
+# included in the handshake request.
+SEC_WEBSOCKET_EXTENSIONS: 7480d3ec-e24a-424a-8148-6527378e9b35
+
 # If a request with this ID comes to the server, the server responds with a string containing all overridden
 # per request settings from the request message. String will be of the form
 # "requestId=19436d9e-f8fc-4b67-8a76-deec60918424 evaluationTimeout=1234, batchSize=12, userAgent=testUserAgent"
diff --git a/gremlin-tools/gremlin-socket-server/src/main/java/org/apache/tinkerpop/gremlin/socket/server/SocketServerSettings.java b/gremlin-tools/gremlin-socket-server/src/main/java/org/apache/tinkerpop/gremlin/socket/server/SocketServerSettings.java
index 08a418dc2d..87a7f04c72 100644
--- a/gremlin-tools/gremlin-socket-server/src/main/java/org/apache/tinkerpop/gremlin/socket/server/SocketServerSettings.java
+++ b/gremlin-tools/gremlin-socket-server/src/main/java/org/apache/tinkerpop/gremlin/socket/server/SocketServerSettings.java
@@ -73,6 +73,12 @@ public class SocketServerSettings {
      */
     public UUID USER_AGENT_REQUEST_ID = null;
 
+    /**
+     * If a request with this ID comes to the server, the server responds with the sec-websocket-extensions header
+     * included in the handshake request.
+     */
+    public UUID SEC_WEBSOCKET_EXTENSIONS = null;
+
     /**
      * If a request with this ID comes to the server, the server responds with a string containing all overridden
      * per request settings from the request message. String will be of the form
diff --git a/gremlin-tools/gremlin-socket-server/src/main/java/org/apache/tinkerpop/gremlin/socket/server/TestWSGremlinInitializer.java b/gremlin-tools/gremlin-socket-server/src/main/java/org/apache/tinkerpop/gremlin/socket/server/TestWSGremlinInitializer.java
index f3a6ce61fd..8f5a9208ae 100644
--- a/gremlin-tools/gremlin-socket-server/src/main/java/org/apache/tinkerpop/gremlin/socket/server/TestWSGremlinInitializer.java
+++ b/gremlin-tools/gremlin-socket-server/src/main/java/org/apache/tinkerpop/gremlin/socket/server/TestWSGremlinInitializer.java
@@ -96,6 +96,7 @@ public class TestWSGremlinInitializer extends TestChannelizers.TestWebSocketServ
     static class ClientTestConfigurableHandler extends MessageToMessageDecoder<BinaryWebSocketFrame> {
         private SocketServerSettings settings;
         private String userAgent = "";
+        private String secWebsocketExtensions = "";
 
         public ClientTestConfigurableHandler(SocketServerSettings settings) { this.settings = settings; }
 
@@ -139,6 +140,8 @@ public class TestWSGremlinInitializer extends TestChannelizers.TestWebSocketServ
                 ctx.channel().writeAndFlush(new CloseWebSocketFrame());
             } else if (msg.getRequestId().equals(settings.USER_AGENT_REQUEST_ID)) {
                 ctx.channel().writeAndFlush(new BinaryWebSocketFrame(returnSimpleBinaryResponse(settings.USER_AGENT_REQUEST_ID, userAgent)));
+            } else if (msg.getRequestId().equals(settings.SEC_WEBSOCKET_EXTENSIONS)) {
+                ctx.channel().writeAndFlush(new BinaryWebSocketFrame(returnSimpleBinaryResponse(settings.SEC_WEBSOCKET_EXTENSIONS, secWebsocketExtensions)));
             } else if (msg.getRequestId().equals(settings.PER_REQUEST_SETTINGS_REQUEST_ID)) {
                 String response = String.format("requestId=%s evaluationTimeout=%d, batchSize=%d, userAgent=%s, materializeProperties=%s",
                         msg.getRequestId(), msg.getArgs().get("evaluationTimeout"),
@@ -186,6 +189,9 @@ public class TestWSGremlinInitializer extends TestChannelizers.TestWebSocketServ
                 if(requestHeaders.contains(USER_AGENT_HEADER)) {
                     userAgent = requestHeaders.get(USER_AGENT_HEADER);
                 }
+                if(requestHeaders.contains("sec-websocket-extensions")) {
+                    secWebsocketExtensions = requestHeaders.get("sec-websocket-extensions");
+                }
                 else {
                     ctx.fireUserEventTriggered(evt);
                 }