Hi,

Pulled TINKERPOP-1767 branch, changed SqlgGraph's io method and ran the tests.

All the io tests are passing.
Only SerializationTest fails for the same reason. It too needs the version specified. I did that locally and then all tests passes.

Thanks
Pieter

On 06/09/2017 18:09, Stephen Mallette wrote:
Pieter, I created this issue:

https://issues.apache.org/jira/browse/TINKERPOP-1767

and made an effort to try to figure a way to fix it:

https://github.com/apache/tinkerpop/tree/TINKERPOP-1767

Note the change to TinkerGraph and its io() method. I suppose you could do
something similar to get the right registry in play? could you have a look
and see if what i did helps? if that works then i'll issue a PR and we can
get it reviewed/merged.


On Tue, Sep 5, 2017 at 12:10 PM, pieter gmail <pieter.mar...@gmail.com>
wrote:

Ok, at present there is only one SimpleModule, the default. I can make it
v2 or v3 but not both.

Lets say I make the SimpleModule support V2.
Then when calling IoEdgeTest for

     {"graphson-v3", true, true,
         (Function<Graph, GraphReader>) g -> g.io(IoCore.graphson()).reader
().mapper(g.io(GraphSONIo.build(GraphSONVersion.V3_0)).
mapper().create()).create(),
         (Function<Graph, GraphWriter>) g -> g.io(IoCore.graphson()).writer
().mapper(g.io(GraphSONIo.build(GraphSONVersion.V3_0)).
mapper().create()).create()},

then the deserializers that run are for V2,

     static class SchemaTableIdJacksonDeserializerV2d0 extends
AbstractObjectDeserializer<SchemaTable> {
         SchemaTableIdJacksonDeserializerV2d0() {
             super(SchemaTable.class);
         }

         @Override
         public SchemaTable createObject(final Map data) {
             return SchemaTable.of((String)data.get("schema"), (String)
data.get("table"));
         }
     }

when createObject fires the map data has V3 elements in it like @type and
@value whilst its expecting "schema" and "table"

If we make the SimpleModule support V3 then graphson-v2 will fail.

    {"graphson-v2", false, false,
        (Function<Graph, GraphReader>) g -> g.io(IoCore.graphson()).reader
().mapper(g.io(GraphSONIo.build(GraphSONVersion.V2_0)).
mapper().typeInfo(TypeInfo.NO_TYPES).create()).create(),
        (Function<Graph, GraphWriter>) g -> g.io(IoCore.graphson()).writer
().mapper(g.io(GraphSONIo.build(GraphSONVersion.V2_0)).
mapper().typeInfo(TypeInfo.NO_TYPES).create()).create()},

Now the deserializers are for V3.

     static class SchemaTableJacksonDeserializerV3d0 extends
StdDeserializer<SchemaTable> {
         public SchemaTableJacksonDeserializerV3d0() {
             super(RecordId.class);
         }

         @Override
         public SchemaTable deserialize(final JsonParser jsonParser, final
DeserializationContext deserializationContext) throws IOException,
JsonProcessingException {
             final Map<String, Object> data = 
deserializationContext.readValue(jsonParser,
Map.class);
             return SchemaTable.of((String)data.get("schema"), (String)
data.get("table"));
         }

         @Override
         public boolean isCachable() {
             return true;
         }
     }

This does not fire at all. Eventually I get a detached edge with an id
that is a map. It never deserialized.

So basically it only works if the SimpleModule version, i.e.
serialize/deserialize code matches up with the version.

Sqlg serializes RecordId to,

     static class RecordIdJacksonSerializerV3d0 extends
StdScalarSerializer<RecordId> {
         public RecordIdJacksonSerializerV3d0() {
             super(RecordId.class);
         }
         @Override
         public void serialize(final RecordId recordId, final JsonGenerator
jsonGenerator, final SerializerProvider serializerProvider)
                 throws IOException, JsonGenerationException {
             final Map<String, Object> m = new HashMap<>();
             m.put("schemaTable", recordId.getSchemaTable());
             m.put("id", recordId.getId());
             jsonGenerator.writeObject(m);
         }
     }

and

     static class SchemaTableJacksonSerializerV3d0 extends
StdScalarSerializer<SchemaTable> {
         SchemaTableJacksonSerializerV3d0() {
             super(SchemaTable.class);
         }

         @Override
         public void serialize(final SchemaTable schemaTable, final
JsonGenerator jsonGenerator, final SerializerProvider serializerProvider)
                 throws IOException, JsonGenerationException {
             // when types are not embedded, stringify or resort to JSON
primitive representations of the
             // type so that non-jvm languages can better interoperate with
the TinkerPop stack.
             final Map<String, Object> m = new LinkedHashMap<>();
             m.put("schema", schemaTable.getSchema());
             m.put("table", schemaTable.getTable());
             jsonGenerator.writeObject(m);
         }

     }

Hope it all makes some sense,
Pieter


On 05/09/2017 17:31, Stephen Mallette wrote:

I guess I'm trying to understand why it matters for purpose of the test.
If
you mix/match versions I can't think of why the test would care one way or
the other. does sqlg serialize its id to a JSON Map?

On Tue, Sep 5, 2017 at 11:19 AM, pieter gmail <pieter.mar...@gmail.com>
wrote:

I looked at TinkerGraph's implementation. In fact I copied it. TinkerGraph
does not have any special id serialization. In fact both its
TinkerIoRegistryV3d0 andTinkerIoRegistryV2d0 registry uses
TinkerModuleV2d0.


In IoCustomTest tests you call g.io(GraphSONIo.build(GraphSON
Version.V2_0)).mapper().addCustomModule(moduleV2d0).
I.e. the test manually registers the appropriate SimpleModule for each
version.

For IoEdgeTest the same needs to happen somehow. Currently I have only
one
default V3 SimpleModule same as TinkerGraph. I have written a V2
SimpleModule but how in IoEdgeTest will the correct IoRegistry or
SimpleModule be selected? The test itself does not call addCustomModule()
and between the builders, mappers, registries and modules I don't see how
to add it.

Thanks,
Pieter

On 05/09/2017 16:30, Stephen Mallette wrote:

You have registries for each version as well and default to v3. Please
see
TinkerGraph:

https://github.com/apache/tinkerpop/blob/master/tinkergraph-
gremlin/src/main/java/org/apache/tinkerpop/gremlin/
tinkergraph/structure/TinkerGraph.java#L198

If the user wants to override that then that's their choice, but they
have
to rig it all up. We probably need a better system than this. IO is way
too
complicated and confusing.

On Tue, Sep 5, 2017 at 9:52 AM, pieter gmail <pieter.mar...@gmail.com>
wrote:

Afraid I still don't quite get it. How do I register the different

SimpleModules depending on the version.

Currently it starts in SqlgGraph,

@Override
public <I extends Io> I io(final Io.Builder<I> builder) {
       return (I) builder.graph(this).onMapper(mapper ->
mapper.addRegistry(SqlgIoRegistry.getInstance())).create();
}

and the SqlgIoRegistry registers the SimpleModule

private SqlgIoRegistry() {
       final SqlgSimpleModule sqlgSimpleModule = new SqlgSimpleModule();
       register(GraphSONIo.class, null, sqlgSimpleModule);
       register(GryoIo.class, RecordId.class, null);
}


Is the SqlgGraph.io(...) method suppose to interrogate the builder to
check the version and add a corresponding IoRegistry and SimpleModule?

Thanks
Pieter






On 05/09/2017 13:30, Stephen Mallette wrote:

I think you should just create a new SimpleModule for each version -

don't
try to put them in the same SqlgSimpleModule. It's a naming convention
that
users will have to follow rather than something explicitly enforced
through
code.

On Sun, Sep 3, 2017 at 3:20 PM, pieter gmail <pieter.mar...@gmail.com
wrote:

Hi,

I am getting IO tests failures on 3.3.0.
Sqlg has a SimpleModule which add serializers for its custom id.

        SqlgSimpleModule() {
            super("custom");
//        addSerializer(RecordId.class, new
RecordId.RecordIdJacksonSerial
izerV2d0());
//        addDeserializer(RecordId.class, new
RecordId.RecordIdJacksonDeserializerV2d0());
//        addSerializer(SchemaTable.class, new
SchemaTable.SchemaTableIdJacksonSerializerV2d0());
//        addDeserializer(SchemaTable.class, new
SchemaTable.SchemaTableIdJacksonDeserializerV2d0());

            addSerializer(RecordId.class, new
RecordId.RecordIdJacksonSerial
izerV3d0());
            addDeserializer(RecordId.class, new
RecordId.RecordIdJacksonDeseri
alizerV3d0());
            addSerializer(SchemaTable.class, new
SchemaTable.SchemaTableJacksonSerializerV3d0());
            addDeserializer(SchemaTable.class, new
SchemaTable.SchemaTableJacksonDeserializerV3d0());
        }

How is it suppose to distinguish between v2 and v3?

An example of a failure is 'IoEdgeTest.shouldReadWriteEdge'

If ...V2d0.. is added to the serializers then 'graphson-v3' fails.
If ...V3d0.. is added to the serializers then 'graphson-v2' fails.

TinkerPop's own CustomId tests do not rely on default behavior and
manually creates SimpleModules for each scenario.

Are they both suppose to work somehow?

Thanks
Pieter





Reply via email to