search

foxcpp / go-jmap

:email: JMAP client & server library (WIP)
MIT License
35 stars 5 forks source link

Add rfc6901 implementation #3

Closed gernest closed 5 years ago

gernest commented 5 years ago

RFC6901

This is used when referencing previous method results https://tools.ietf.org/html/draft-ietf-jmap-core-17#section-3.6

OFFTOPIC : I just discovered this project today, I have been working on jmap go implementation too.I was more focused on the server, I see you focused more on the client. I will try go integrate my server code into this, I don't think its necessary to duplicate effort.

foxcpp commented 5 years ago

https://travis-ci.com/foxcpp/go-jmap/jobs/231497768

json_ponter.go:48:10: v.MapRange undefined (type reflect.Value has no field or method MapRange) (typecheck)
        r := v.MapRange()
               ^

This function was added in Go 1.12, Can you update .travisci.yml to use that version?

We can ignore failed build on Go 1.13, there is known bug in typecheck.

foxcpp commented 5 years ago

OFFTOPIC : I just discovered this project today, I have been working on jmap go implementation too.I was more focused on the server, I see you focused more on the client. I will try go integrate my server code into this, I don't think its necessary to duplicate effort.

I really want to get JMAP support into maddy so that would be very nice. Created an issue for server implementation discussion (#4), you can also PM me on freenode IRC (foxcpp) if you want to talk about server design in general.

gernest commented 5 years ago

@foxcpp

This function was added in Go 1.12, Can you update .travisci.yml to use that version?

Done.

I really want to get JMAP support into maddy so that would be very nice. Created an issue for server implementation discussion (#4), you can also PM me on freenode IRC (foxcpp) if you want to talk about server design in general.

I am not familiar with maddy yet, but from the issue you linked its clear that go-jmap will be generic so backend specific details can be handled separately. I mean more like a library from which someone can assemble a functioning jmap server.

I'm not familiar with irc will try to chat after googling my way through it.

My current jmap efforts were more as a server and not a library for implementing jmap servers, I will try to pickup good parts and apply them to go-jmap

foxcpp commented 5 years ago

Provided that this PR does not implement JMAPs extension to the JSON Pointer (* for mapping through array), why do you think it is necessary to roll your own implementation instead of using any of the other publicly available implementations?

Also, some information about original JSON object is lost when it is converted to a structure. Especially, if there is a custom UnmarshalJSON function or struct tags (we use a lot of them, but they doesn't seem to be supported by your code). I think it is necessary to operate on the JSON data model level (e.g. on map[string]interface{}) instead of already translated structure object.

Also, I have concerns about performance due to a heavy use of reflection in this PR. Since JSON pointer resolution is going to end up in the "hot" code for requests processing, this is important to us.

gernest commented 5 years ago

I see the project you linked meets your standards. I'm closing this in favour of the linked project.

I was using this to get familiar with your project.