Which tests in particular are failing for SerializationTest? all of them? On Wed, Sep 6, 2017 at 1:35 PM, pieter gmail <pieter.mar...@gmail.com> wrote:
> 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 >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >>>>>>>>> >
