Skip to content
This repository was archived by the owner on Aug 9, 2018. It is now read-only.

Test that the JSON coding works #16

Merged
merged 1 commit into from
Nov 2, 2015
Merged

Conversation

mildred
Copy link
Contributor

@mildred mildred commented Oct 29, 2015

I tried to test that decoding JSON actually worked and we could get links from it, and it failed. The problem was that the link object was a map[string]interface{} and not a Node. This was the issue described in issue #3, perhaps we should reopen it as there is likely a similar issue with cbor.

Add a test that decodes a JSON object, and detect links in it. Then encode
it again and it must match the original file. The test failed (because the
link was a map[string]interface{} instead of a Node). The fix post-process
the result of the JSON decoder to convert map[string]interface{} to Node.
@jbenet
Copy link
Contributor

jbenet commented Nov 1, 2015

ah, thanks @mildred -- yes we should test cbor too. sad that we have to run another pass through the whole thing. the ugorji codec used to let you define the type-- do we know if the json codec can do that?

@mildred
Copy link
Contributor Author

mildred commented Nov 2, 2015

We cannot directly.

What we can do is parse the JSON as a token stream, and construct whatever data structure we want, but it is a bit more involved and does not adheres the multicodec interface Decode(value interface{}) (from memory).

The json package also let us declare our type to be a Unmarshaller (see doc) but I don't know how we can make this work recursively. The UnmarshalJSON function takes a byte array as argument that must be further decoded. The problem in our case is that we would like to decode it to an interface{} but that means letting the json library choose the exact type used.

There is an issue that talks about something similar to the Unmarshaller interface, but for token stream instead golang/go#12001. Unfortunately the Go language is just in feature freeze for 3 months now.

@jbenet
Copy link
Contributor

jbenet commented Nov 2, 2015

:/ sucks. definitely throws off our IPLD groove.

I think the cbor marshaller does allow this-- maybe im totally off. if not, we should look into adding it. parsing json is slow anyway, and this convert step wont be a big deal, but in the cbor case, we want that to be as fast as possible. (json too, but ok to slack a bit for while)

@jbenet
Copy link
Contributor

jbenet commented Nov 2, 2015

@whyrusleeping keep an eye on all these issues as IPLD is close to landing. dont want to be caught unaware

jbenet added a commit that referenced this pull request Nov 2, 2015
Test that the JSON coding works
@jbenet jbenet merged commit 74577d3 into ipld:master Nov 2, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants