search

multiformats / rust-cid

CID in rust
90 stars 49 forks source link

Add support for string deserialization with serde #162

Closed jmg-duarte closed 3 months ago

jmg-duarte commented 3 months ago

I'm working on a CLI tool that needs to read CIDs in JSONs but the current implementation does not support visit_str, though Cid::try_from already supports reading from &str.

I don't understand why CID isn't compatible with Serde's data model, the reason isn't documented either: https://github.com/multiformats/rust-cid/blob/dae650cc5a9778e4320bfe8cefbb214b67a9ad29/src/serde.rs#L1-L6

I tried my hand at doing something about it since the Cid::try_from + visit_str are right at hand but failed miserably, getting stumped by the MainEntryVisitor. https://github.com/multiformats/rust-cid/blob/dae650cc5a9778e4320bfe8cefbb214b67a9ad29/src/serde.rs#L83-L105

I'm keen on discussing more and implementing a solution.

vmx commented 3 months ago

The idea of the rust-cid Serde implementation is that it works well with IPLD serialization format implementations like https://github.com/ipld/serde_ipld_dagcbor and https://github.com/ipld/serde_ipld_dagjson. This is where the "Serde data model doesn't match the IPLD data model" come into play.

Let's take DAG-JSON as an example and let's assume we don't do anything special about the CIDs. We would parse the a CID encoded as {"/": "mycid"} as "map with slash as key and "mycid" as a value" into the Serde data model. When we then serialize it to DAG-JSON it would work. But if we now serialize it to e.g. DAG-CBOR it would end up with that map and not with a CID. Of course we could special case the DAG-CBOR serializer and say if it encounters such a map, it serializes as a CID.

But what if the input was DAG-CBOR and there was a map with a slash and a string? We would parse it, but when we serialize it again, it would serialized as a CID. That's the reason why we need to treat CIDs in a special way. Using a Newtype-Struct is the usual way of doing that in Serde.

jmg-duarte commented 3 months ago

Hmmm I see. So no clear cut solution asides from implementing a custom wrapper on my end?

vmx commented 3 months ago

Yes, it sounds like your own custom wrapper would make sense. As it sounds like a non DAG-JSON json format, it might also be valuable to look into https://github.com/ipld/serde_ipld_dagjson and see if such a wrapper around a serde JSON parser might make sense.

jmg-duarte commented 3 months ago

Alright, closing then. Thanks for the help!