Should we provide a more general VertexSerializer in GraphBinaryMessageSerializerV1 to compatible with GraphSONMessageSerializerV3d0

[Stark Arya <sa...@gmail.com>]

In TinkerPop， the client request - response path is:    client-req  -->   server-generateResponse  --> server-response-serializer --> client-response-deserializer -> client-respone-ogm

Two key steps:
1. server-generateResponse:   
    TinkerPop server could  generate results with: ReferenceVertex, ReferenceEdge in most scenarios to reduce network package size and reduce seralizer-deseralizer time,  this works well。
    But in very limited scenarios like people themselves know the result set is few enough (or by limit step) and  people need DetachedVertex, DetachedEdge,..., to get all information in a driect way。
      TinkerPop also provide DetachedFactory to detach any result XXX into detachedXXX, this is good and the Tinkerpop server could generate DetachedXXX in response easily。

2. server-response-serializer / client-response-deserializer: 
     For GraphSONMessageSerializerV3d0,  If server-generateResponse  return ReferenceVertex,  client will get DetachedVertex but properties will be set null；If server-generateResponse  
          return DetachedVertex,  client will get DetachedVertex with properties will not be null。it seem ok and I think This model works well： TinkerPop Server decide if return ReferenceXXX or
          DetachedXXX。serializer / deserializer just transfer the result， Serializer / deserializer should not interfere the server of how the response format should be which will make the system complex。

     But For GraphBinaryMessageSerializerV1， the upper constraint is destroyed。Even  server-generateResponse generate DetachedVertex the client will get ReferenceVertex eventually 。 
     I do not think this is a good choice。It will make TinkerPop server unable to control client’s experience. to cover this problem, TinkerPop server provider should extend Serializer by the following code：
   
writer = new GraphBinaryWriter(TypeSerializerRegistry.build()
        .addCustomType(DetachedVertex.class, new DetachedVertexSerializer())
        .addCustomType(DetachedEdge.class, new DetachedEdgeSerializer())
        .addCustomType(DetachedPath.class, new DetachedPathSerializer())
        .create());

reader = new GraphBinaryReader(TypeSerializerRegistry.build()
        .addCustomType(DetachedVertex.class, new DetachedVertexSerializer())
        .addCustomType(DetachedEdge.class, new DetachedEdgeSerializer())
        .addCustomType(DetachedPath.class, new DetachedPathSerializer())
        .create());

   This need reconstruct  gremlin-driver which will be incompatible with the standard gremlin-driver and will take Inconvenience to users；

   So I think we should take careful consideration with properties as the annotations have already been mentioned， and judge Vertex  type when writeValue to determine if write properties ?
protected Vertex readValue(final ByteBuf buffer, final GraphBinaryReader context) throws SerializationException {
    final Vertex v = new ReferenceVertex(context.read(buffer), 
                                         context.readValue(buffer, String.class, false));
    
    // discard the properties - as we only send "references" this should always be null, but will we change our
    // minds some day????
    context.read(buffer);
    return v;
}
@Override
protected void writeValue(final Vertex value, final ByteBuf buffer, final GraphBinaryWriter context) throws SerializationException {
    context.write(value <http://value.id/>. <http://value.id/>id <http://value.id/>(), buffer);
    context.writeValue(value.label(), buffer, false);
    context.write(null, buffer);
}

Any comments are very welcome, Thanks!