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


##########
gremlin-driver/src/main/java/org/apache/tinkerpop/gremlin/driver/handler/HttpGremlinRequestEncoder.java:
##########
@@ -81,7 +90,9 @@ protected void encode(final ChannelHandlerContext 
channelHandlerContext, final R
             request.headers().add(HttpHeaderNames.CONTENT_TYPE, 
"application/json");
             request.headers().add(HttpHeaderNames.CONTENT_LENGTH, 
payload.length);
             request.headers().add(HttpHeaderNames.ACCEPT, 
serializer.mimeTypesSupported()[0]);
-
+            if (userAgentEnabled) {
+                request.headers().add(UserAgent.USER_AGENT_HEADER_NAME, 
UserAgent.USER_AGENT);

Review Comment:
   `UserAgent.USER_AGENT_HEADER_NAME` (`"User-Agent"`) matches the header used 
in websocket requests.
   `HttpHeaderNames.USER_AGENT` (`"user-agent"`) may be preferable if we are ok 
with the inconsistency.



##########
docs/src/upgrade/release-3.6.x.asciidoc:
##########
@@ -33,6 +33,9 @@ complete list of all the modifications that are part of this 
release.
 
 === Upgrading for Providers
 
+The `gremlinRequestEncoder` constructor has been deprecated in favor of one 
with an additional parameter `boolean userAgentEnabled`.

Review Comment:
   ```suggestion
   The `HttpGremlinRequestEncoder` constructor has been deprecated in favor of 
one with an additional parameter `boolean userAgentEnabled`.
   ```



##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/WsAndHttpChannelizerHandler.java:
##########
@@ -62,28 +63,35 @@ public void configure(final ChannelPipeline pipeline) {
     public void channelRead(final ChannelHandlerContext ctx, final Object obj) 
{
         final ChannelPipeline pipeline = ctx.pipeline();
         if (obj instanceof HttpMessage && 
!WebSocketHandlerUtil.isWebSocket((HttpMessage)obj)) {
-            // if the message is for HTTP and not websockets then this handler 
injects the endpoint handler in front
-            // of the HTTP Aggregator to intercept the HttpMessage. Therefore 
the pipeline looks like this at start:
+            // If the message is for HTTP and not WS then this handler injects 
the HTTP user-agent and endpoint handlers
+            // in front of the HTTP aggregator to intercept the HttpMessage.
+            // This replaces the WS server protocol handler so that the 
pipeline initially looks like this:
             //
             // IdleStateHandler -> HttpResponseEncoder -> HttpRequestDecoder ->
             //    WsAndHttpChannelizerHandler -> HttpObjectAggregator ->
+            //    WebSocketServerProtocolHandler ->

Review Comment:
   Was `WebSocketServerProtocolHandler` mistakenly left out of the original 
comment?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to