You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "kenhuuu (via GitHub)" <gi...@apache.org> on 2024/04/27 06:36:03 UTC
[PR] Create MessageSerializerV4s and remove MessageSerializer. [tinkerpop]
kenhuuu opened a new pull request, #2576:
URL: https://github.com/apache/tinkerpop/pull/2576
This removes all the old MessageSerializers which includes GraphBinaryv1 and GraphSONv1-3.
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create MessageSerializerV4s and remove MessageSerializer. [tinkerpop]
Posted by "kenhuuu (via GitHub)" <gi...@apache.org>.
kenhuuu merged PR #2576:
URL: https://github.com/apache/tinkerpop/pull/2576
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create MessageSerializerV4s and remove MessageSerializer. [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on code in PR #2576:
URL: https://github.com/apache/tinkerpop/pull/2576#discussion_r1583706735
##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java:
##########
@@ -156,36 +153,20 @@ else if (accept.equals("text/plain")) {
* Request body contains serialized RequestMessage
*/
public RequestMessageV4 getRequestMessageFromHttpRequest(final FullHttpRequest request,
- Map<String, MessageSerializer<?>> serializers) throws SerializationException {
+ Map<String, MessageSerializerV4<?>> serializers) throws SerializationException {
final String contentType = request.headers().get(HttpHeaderNames.CONTENT_TYPE);
- if (request.method() == POST && contentType != null && !contentType.equals("application/json") && serializers.containsKey(contentType)) {
- final MessageSerializer<?> serializer = serializers.get(contentType);
- if (!(serializer instanceof MessageTextSerializerV4)) {
- throw new SerializationException("Server only supports V4 or later serializers.");
+ if (contentType != null && !contentType.equals("application/json") && serializers.containsKey(contentType)) {
+ final MessageSerializerV4<?> serializer = serializers.get(contentType);
+ if (!(serializer instanceof MessageSerializerV4)) {
Review Comment:
not needed
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create MessageSerializerV4s and remove MessageSerializer. [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on code in PR #2576:
URL: https://github.com/apache/tinkerpop/pull/2576#discussion_r1583704655
##########
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/handler/HttpRequestMessageDecoder.java:
##########
@@ -156,36 +153,20 @@ else if (accept.equals("text/plain")) {
* Request body contains serialized RequestMessage
*/
public RequestMessageV4 getRequestMessageFromHttpRequest(final FullHttpRequest request,
- Map<String, MessageSerializer<?>> serializers) throws SerializationException {
+ Map<String, MessageSerializerV4<?>> serializers) throws SerializationException {
final String contentType = request.headers().get(HttpHeaderNames.CONTENT_TYPE);
- if (request.method() == POST && contentType != null && !contentType.equals("application/json") && serializers.containsKey(contentType)) {
- final MessageSerializer<?> serializer = serializers.get(contentType);
- if (!(serializer instanceof MessageTextSerializerV4)) {
- throw new SerializationException("Server only supports V4 or later serializers.");
+ if (contentType != null && !contentType.equals("application/json") && serializers.containsKey(contentType)) {
+ final MessageSerializerV4<?> serializer = serializers.get(contentType);
+ if (!(serializer instanceof MessageSerializerV4)) {
+ throw new SerializationException("Server only supports MessageSerializerV4 or later serializers.");
}
- final MessageTextSerializerV4<?> serializerV4 = (MessageTextSerializerV4) serializer;
-
+ final MessageSerializerV4<?> serializerV4 = serializer;
Review Comment:
variable can be removed
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create MessageSerializerV4s and remove MessageSerializer. [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on code in PR #2576:
URL: https://github.com/apache/tinkerpop/pull/2576#discussion_r1583698290
##########
gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ClusterConfigTest.java:
##########
@@ -33,40 +32,21 @@
public class ClusterConfigTest {
@Test
- public void shouldPropagateSerializerConstraintsForGraphSON3() {
+ public void shouldPropagateSerializerConstraintsForGraphSON4() {
final Configuration config = new BaseConfiguration();
config.setProperty("serializer.config.maxNumberLength", 999);
config.setProperty("serializer.config.maxStringLength", 123456);
config.setProperty("serializer.config.maxNestingDepth", 55);
config.setProperty("hosts", Arrays.asList("localhost"));
- config.setProperty("serializer.className", GraphSONMessageSerializerV3.class.getCanonicalName());
+ config.setProperty("serializer.className", GraphSONMessageSerializerV4.class.getCanonicalName());
final Cluster cluster = Cluster.open(config);
- assertTrue(cluster.getSerializer() instanceof GraphSONMessageSerializerV3);
- final GraphSONMessageSerializerV3 serV3 = (GraphSONMessageSerializerV3) cluster.getSerializer();
+ assertTrue(cluster.getSerializer() instanceof GraphSONMessageSerializerV4);
+ final GraphSONMessageSerializerV4 serV3 = (GraphSONMessageSerializerV4) cluster.getSerializer();
Review Comment:
nit: var name
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Create MessageSerializerV4s and remove MessageSerializer. [tinkerpop]
Posted by "vkagamlyk (via GitHub)" <gi...@apache.org>.
vkagamlyk commented on code in PR #2576:
URL: https://github.com/apache/tinkerpop/pull/2576#discussion_r1583730784
##########
gremlin-util/src/main/java/org/apache/tinkerpop/gremlin/util/ser/SerializersV4.java:
##########
@@ -18,50 +18,20 @@
*/
package org.apache.tinkerpop.gremlin.util.ser;
-import org.apache.tinkerpop.gremlin.util.MessageSerializer;
+import org.apache.tinkerpop.gremlin.util.MessageSerializerV4;
/**
* An enum of the default serializers.
*
* @author Stephen Mallette (http://stephen.genoprime.com)
*/
-public enum Serializers {
+public enum SerializersV4 {
/**
* GraphSON 3.0.
Review Comment:
```suggestion
* GraphSON 4.0.
```
--
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: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org