pieter, i updated that branch and issued this PR (and i'm running the full test suite now):
https://github.com/apache/tinkerpop/pull/706 please review/VOTE if i got it right - let's finish off any final discussion/tweaks on the PR itself. thanks for noting this problem. > This v3 looks like a <3 heart where the top broke off. hehe On Wed, Sep 6, 2017 at 1:56 PM, Paul A. Jackson <paul.jack...@pb.com> wrote: > This v3 looks like a <3 heart where the top broke off. > > Sad. > > > -----Original Message----- > From: Stephen Mallette [mailto:spmalle...@gmail.com] > Sent: Wednesday, September 06, 2017 1:45 PM > To: dev@tinkerpop.apache.org > Subject: Re: io and graphson-v3 > > never mind - i see what's messed up > > On Wed, Sep 6, 2017 at 1:43 PM, Stephen Mallette <spmalle...@gmail.com> > wrote: > > > 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 > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >>>>>>>>>> > >> > > >
