It makes sense that we shouldn't carry it forward. That wasn't what I was
getting to though. My initial plea was that since moving from GraphSON 1.0
to 3.0 can introduce some breaking changes I was asking if this should be
documented somewhere (update notes for example).
I've since decided against it, because this really only affects non-java
users (who typically would not be aware of the underlying IO but would be
affected by the changes) and depending on the driver's implementation it
may or may not be breaking. So it should be up to the "client" to report
the breaking change, not TP.

Right now since the php driver still needs to support the 3.2.x line, I've
opted to deserialize into the old ugly format. I'll introduce a breaking
change later.






On Mon, Apr 16, 2018 at 7:36 PM, Stephen Mallette <spmalle...@gmail.com>
wrote:

> the "1" is no longer a thing (i.e. the toString() of a complex object)
> because GraphSON 3.0 allows complex keys. That "1" was a hack in prior
> versions of GraphSON to get around that problem. We shouldn't be trying to
> carry that forward. Does that make sense?
>
> On Mon, Apr 16, 2018 at 4:17 PM, Dylan Millikin <dylan.milli...@gmail.com>
> wrote:
>
> > Actually, let me change that up a bit. I would expect the Map to be part
> of
> > Tree, not encapsulate it. So something like :
> >
> > "data":{
> >    "@type":"g:List",
> >    "@value":[{
> >       "@type":"g:Tree",
> >       "@value":[{
> >             "@type":"g:Map",
> >             "@value":{
> >                "1",
> >                {
> >                   "key":{
> >                      "@type":"g:Vertex",
> >                      "@value":{
> >                         "id":{"@type":"g:Int64","@value":1},
> >                         "label":"vertex",
> >                         "properties":{"name": ...
> >
> > It could be debated that this is something the client side serializer
> > could/would need to add in when treating Tree. But still a little
> lopsided.
> >
> > BTW if you're wondering, the "1" key from GRAPHSON 1.0 is a string
> > representation of the *element id*. So I really wouldn't be surprised if
> > people were using that information currently.
> >
> >
> > On Mon, Apr 16, 2018 at 4:04 PM, Dylan Millikin <
> dylan.milli...@gmail.com>
> > wrote:
> >
> > > I'm going to leverage this email a bit more for the following point.
> > >
> > > I noticed that the actual format of Tree has changed with GRAPHSON 3.0.
> > > We're now handling Lists whereas it used to be Maps. This seems to be a
> > > breaking change when using GRAPHSON 3.0.
> > >
> > > Given: g.V(1).out().properties("name").tree()
> > >
> > > In 1.0 we get :
> > >
> > > "data":[{
> > >    "1":{
> > >       "key":{
> > >          "id":1,
> > >          "label":"vertex",
> > >          "type":"vertex",
> > >          "properties":{"name": ...
> > >
> > > In 3.0 we get :
> > >
> > > "data":{
> > >    "@type":"g:List",
> > >    "@value":[{
> > >       "@type":"g:Tree",
> > >       "@value":[{
> > >          "key":{
> > >          "@type":"g:Vertex",
> > >          "@value":{
> > >              "id":{"@type":"g:Int64","@value":1},
> > >              "label":"vertex",
> > >              "properties":{"name": ...
> > >
> > > When instead we should be getting :
> > >
> > > "data":{
> > >    "@type":"g:Map",
> > >    "@value":[{
> > >       "1",
> > >       {
> > >          "@type":"g:Tree",
> > >          "@value":[{
> > >             "key":{
> > >                "@type":"g:Vertex",
> > >                "@value":{
> > >                    "id":{"@type":"g:Int64","@value":1},
> > >                    "label":"vertex",
> > >                    "properties":{"name": ...
> > >
> > > I like the new way better to be honest. And it's probably more
> consistent
> > > with gryo, but this is a breaking change and inconsistent across
> > > serializing methods ATM. At the very least we should add it to the
> > upgrade
> > > information and perhaps consider aligning GRAPHSON 1.0 with this for
> 3.4
> > >
> > > Let me know if you guys think it deserves a thread of it's own. I just
> > > didn't want to pollute the list too much while I fiddle around.
> > >
> > > On Mon, Apr 16, 2018 at 11:27 AM, Dylan Millikin <
> > dylan.milli...@gmail.com
> > > > wrote:
> > >
> > >> Ok that makes a lot of sense. I’ll change the the way the driver
> > operates
> > >> to reflect that and I’ll just have the RequestMessage serialize down
> to
> > a
> > >> native json map.
> > >>
> > >> Cheers!
> > >>
> > >> On Mon 16 Apr 2018 at 07:36, Stephen Mallette <spmalle...@gmail.com>
> > >> wrote:
> > >>
> > >>> The IO examples show that it works as you say:
> > >>>
> > >>> http://tinkerpop.apache.org/docs/current/dev/io/#_requestmessage_3
> > >>>
> > >>> I'd agree that it isn't really consistent though the "type" really
> > isn't
> > >>> a
> > >>> g:Map i don't think - it's really a RequestMessage, but we don't
> have a
> > >>> specific type for that. Gremlin Server just understands it that way.
> > >>>
> > >>>
> > >>>
> > >>> On Mon, Apr 16, 2018 at 1:51 AM, Dylan Millikin <dm...@apache.org>
> > >>> wrote:
> > >>>
> > >>> >  Hey guys,
> > >>> >
> > >>> > Just been working on implementing GRAPHSON 3.0 into the php world.
> > >>> (testing
> > >>> > against gremlin-server 3.3.2)
> > >>> >
> > >>> > I noticed that the following request message fails:
> > >>> >
> > >>> > {
> > >>> >    "@type":"g:Map",
> > >>> >    "@value":[
> > >>> >       "requestId","f990037e-3b55-49a4-a108-2f0e8c162715",
> > >>> >       "processor","",
> > >>> >       "op","eval",
> > >>> >       "args",{"@type":"g:Map","@value":["gremlin","5+5"]}
> > >>> >    ]
> > >>> > }
> > >>> >
> > >>> > While this one doesn't:
> > >>> >
> > >>> > {
> > >>> >    "requestId":"bf26426b-ba27-475f-aafa-527ce0b0116c",
> > >>> >    "processor":"",
> > >>> >    "op":"eval",
> > >>> >    "args":{"@type":"g:Map","@value":["gremlin","5+5"]}
> > >>> >    }
> > >>> > }
> > >>> >
> > >>> > Seems like the parent map should not be serialized using GRAPHSON
> 3.0
> > >>> > standards. This isn't readily apparent because going through the
> > code I
> > >>> > noticed both gremlin-javascript and gremlin-python seem to simply
> > fall
> > >>> back
> > >>> > to JSON natives for this task and don't define the map type? PHP is
> > >>> very
> > >>> > much the same and I could fall back to JSON natives, but is this an
> > >>> > oversight? You would expect the entire "message" portion of the
> > >>> > Websocket RequestMessage
> > >>> > to be serialized by the same standard.
> > >>> >
> > >>> > I've overall done a decent job of staying up to date with the
> mailing
> > >>> list
> > >>> > (even though I haven't participated) but I may have overlooked this
> > >>> already
> > >>> > being discussed. If anyone has a tldr regarding this that would be
> > >>> great?
> > >>> >
> > >>> > Thanks!
> > >>> >
> > >>> > FYI : error message for the first one, it seems to choke on the
> > @value
> > >>> > List.
> > >>> >
> > >>> > [WARN] AbstractGraphSONMessageSerializerV2d0 - Request
> > >>> > [PooledUnsafeDirectByteBuf(ridx: 158, widx: 158, cap: 175)] could
> > not
> > >>> be
> > >>> > deserialized by org.apache.tinkerpop.gremlin.driver.ser.
> > >>> > AbstractGraphSONMessageSerializerV2d0.
> > >>> > org.apache.tinkerpop.shaded.jackson.databind.JsonMappingException:
> > >>> Could
> > >>> > not deserialize the JSON value as required. Nested exception:
> > >>> > java.lang.InstantiationException:
> > >>> > Cannot deserialize the value with the detected type contained in
> the
> > >>> JSON
> > >>> > ('g:Map') to the type specified in parameter to the object mapper
> > >>> (class
> > >>> > org.apache.tinkerpop.gremlin.driver.message.RequestMessage). Those
> > >>> types
> > >>> > are incompatible.
> > >>> >  at [Source: (byte[])"{"@type":"g:Map","@va
> > >>> lue":["requestId","f990037e-
> > >>> > 3b55-49a4-a108-2f0e8c162715","processor","","op","eval","
> > >>> > args",{"@type":"g:Map","@value":["gremlin","5+5"]}]}"; line: 1,
> > >>> column:
> > >>> > 27]
> > >>> >
> > >>>
> > >>
> > >
> >
>

Reply via email to