You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by jo...@apache.org on 2019/05/08 11:35:47 UTC

[tinkerpop] branch master updated: Correct sample custom serializer for GraphBinary CTR

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

jorgebg pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git


The following commit(s) were added to refs/heads/master by this push:
     new 25e185f  Correct sample custom serializer for GraphBinary CTR
25e185f is described below

commit 25e185f95426bd812b727b57620eb894a7eee1ef
Author: Jorge Bay Gondra <jo...@gmail.com>
AuthorDate: Wed May 8 13:35:39 2019 +0200

    Correct sample custom serializer for GraphBinary CTR
---
 .../types/sample/SamplePersonSerializer.java       | 64 ++++++++++++++--------
 .../types/sample/SamplePersonSerializerTest.java   | 22 ++++++++
 2 files changed, 62 insertions(+), 24 deletions(-)

diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/sample/SamplePersonSerializer.java b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/sample/SamplePersonSerializer.java
index 7efe0ea..03b54c3 100644
--- a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/sample/SamplePersonSerializer.java
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/sample/SamplePersonSerializer.java
@@ -25,6 +25,7 @@ import org.apache.tinkerpop.gremlin.driver.ser.binary.GraphBinaryReader;
 import org.apache.tinkerpop.gremlin.driver.ser.binary.GraphBinaryWriter;
 import org.apache.tinkerpop.gremlin.driver.ser.binary.types.CustomTypeSerializer;
 
+import java.nio.charset.StandardCharsets;
 import java.util.Date;
 
 /**
@@ -51,13 +52,29 @@ public final class SamplePersonSerializer implements CustomTypeSerializer<Sample
             throw new SerializationException("{custom_type_info} should not be provided for this custom type");
         }
 
-        final byte valueFlag = buffer.readByte();
-        if ((valueFlag & 1) == 1) {
-            return null;
+        return readValue(buffer, context, true);
+    }
+
+    @Override
+    public SamplePerson readValue(final ByteBuf buffer, final GraphBinaryReader context, final boolean nullable) throws SerializationException {
+        if (nullable) {
+            final byte valueFlag = buffer.readByte();
+            if ((valueFlag & 1) == 1) {
+                return null;
+            }
         }
 
-        // Read the buffer int, no necessary in this case
-        buffer.readInt();
+        // Read the byte length of the value bytes
+        final int valueLength = buffer.readInt();
+
+        if (valueLength <= 0) {
+            throw new SerializationException(String.format("Unexpected value length: %d", valueLength));
+        }
+
+        if (valueLength > buffer.readableBytes()) {
+            throw new SerializationException(
+                String.format("Not enough readable bytes: %d (expected %d)", valueLength, buffer.readableBytes()));
+        }
 
         final String name = context.readValue(buffer, String.class, false);
         final Date birthDate = context.readValue(buffer, Date.class, false);
@@ -66,35 +83,34 @@ public final class SamplePersonSerializer implements CustomTypeSerializer<Sample
     }
 
     @Override
-    public SamplePerson readValue(final ByteBuf buffer, final GraphBinaryReader context, final boolean nullable) throws SerializationException {
-        throw new SerializationException("SamplePersonSerializer can not read a value without type information");
+    public void write(final SamplePerson value, final ByteBuf buffer, final GraphBinaryWriter context) throws SerializationException {
+        // Write {custom type info}, {value_flag} and {value}
+        buffer.writeBytes(typeInfoBuffer);
+
+        writeValue(value, buffer, context, true);
     }
 
     @Override
-    public void write(final SamplePerson value, final ByteBuf buffer, final GraphBinaryWriter context) throws SerializationException {
+    public void writeValue(final SamplePerson value, final ByteBuf buffer, final GraphBinaryWriter context, final boolean nullable) throws SerializationException {
         if (value == null) {
-            buffer.writeBytes(typeInfoBuffer);
+            if (!nullable) {
+                throw new SerializationException("Unexpected null value when nullable is false");
+            }
+
             context.writeValueFlagNull(buffer);
             return;
         }
 
-        buffer.writeBytes(typeInfoBuffer);
-        context.writeValueFlagNone(buffer);
+        if (nullable) {
+            context.writeValueFlagNone(buffer);
+        }
 
-        final ByteBuf valueBuffer = buffer.alloc().buffer();
-        try {
-            context.writeValue(value.getName(), valueBuffer, false);
-            context.writeValue(value.getBirthDate(), valueBuffer, false);
+        final String name = value.getName();
 
-            buffer.writeInt(valueBuffer.readableBytes());
-            buffer.writeBytes(valueBuffer);
-        } finally {
-            valueBuffer.release();
-        }
-    }
+        // value_length = name_byte_length + name_bytes + long
+        buffer.writeInt(4 + name.getBytes(StandardCharsets.UTF_8).length + 8);
 
-    @Override
-    public void writeValue(final SamplePerson value, final ByteBuf buffer, final GraphBinaryWriter context, final boolean nullable) throws SerializationException {
-        throw new SerializationException("SamplePersonSerializer can not write a value without type information");
+        context.writeValue(name, buffer, false);
+        context.writeValue(value.getBirthDate(), buffer, false);
     }
 }
diff --git a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/sample/SamplePersonSerializerTest.java b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/sample/SamplePersonSerializerTest.java
index f72fbb2..b8cfffa 100644
--- a/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/sample/SamplePersonSerializerTest.java
+++ b/gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/binary/types/sample/SamplePersonSerializerTest.java
@@ -24,6 +24,8 @@ import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
 import org.apache.tinkerpop.gremlin.driver.ser.GraphBinaryMessageSerializerV1;
 import org.apache.tinkerpop.gremlin.driver.ser.SerializationException;
 import org.apache.tinkerpop.gremlin.driver.ser.binary.GraphBinaryIo;
+import org.apache.tinkerpop.gremlin.driver.ser.binary.GraphBinaryReader;
+import org.apache.tinkerpop.gremlin.driver.ser.binary.GraphBinaryWriter;
 import org.apache.tinkerpop.gremlin.driver.ser.binary.TypeSerializerRegistry;
 import org.apache.tinkerpop.gremlin.structure.io.AbstractIoRegistry;
 import org.junit.Test;
@@ -73,6 +75,26 @@ public class SamplePersonSerializerTest {
         assertPerson(serializer);
     }
 
+    @Test
+    public void readValueAndWriteValueShouldBeSymmetric() throws SerializationException {
+        final TypeSerializerRegistry registry = TypeSerializerRegistry.build()
+                .addCustomType(SamplePerson.class, new SamplePersonSerializer()).create();
+        final GraphBinaryReader reader = new GraphBinaryReader(registry);
+        final GraphBinaryWriter writer = new GraphBinaryWriter(registry);
+
+        final SamplePerson person = new SamplePerson("Matias",
+                Date.from(LocalDateTime.of(2005, 8, 5, 1, 0).toInstant(ZoneOffset.UTC)));
+
+        for (boolean nullable: new boolean[] { true, false }) {
+            final ByteBuf buffer = allocator.buffer();
+            writer.writeValue(person, buffer, nullable);
+            final SamplePerson actual = reader.readValue(buffer, SamplePerson.class, nullable);
+
+            assertThat(actual, new ReflectionEquals(person));
+            buffer.release();
+        }
+    }
+
     private void assertPerson(final GraphBinaryMessageSerializerV1 serializer) throws SerializationException {
         final Date birthDate = Date.from(LocalDateTime.of(2010, 4, 29, 5, 30).toInstant(ZoneOffset.UTC));
         final SamplePerson person = new SamplePerson("Olivia", birthDate);