ipld / serde_ipld_dagcbor

DAG-CBOR implementation for Serde
Other
8 stars 8 forks source link

question: when is collect_map called? #26

Closed goeo- closed 6 months ago

goeo- commented 7 months ago

I'm using serde_transcode to deserialize from serde_wasm_bindgen and serialize into dag-cbor with serde_ipld_dagcbor (I had to patch the library to make the Serializer class public), but JavaScript objects, which I've verified to go through serialize_map, never seem to call collect_map to sort the keys and have the output be a valid dag-cbor.

When is this function supposed to be called? serde's documentation seems pretty unclear as well.

vmx commented 7 months ago

It's called whenever serialize is called on a BTreeMap: https://github.com/serde-rs/serde/blob/00c4b0cef80557c33fbcd75fcc70dc034720b4df/serde/src/ser/impls.rs#L431-L481

goeo- commented 7 months ago

Hmm are regular maps not supposed to be key sorted?

On Mon 4. Mar 2024 at 17:32, Volker Mische @.***> wrote:

It's called whenever serialize is called on a BTreeMap: https://github.com/serde-rs/serde/blob/00c4b0cef80557c33fbcd75fcc70dc034720b4df/serde/src/ser/impls.rs#L431-L481

— Reply to this email directly, view it on GitHub https://github.com/ipld/serde_ipld_dagcbor/issues/26#issuecomment-1976267155, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABSXZAPUPFMJ5DJRJF245JDYWREVXAVCNFSM6AAAAABEDP5DZKVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTSNZWGI3DOMJVGU . You are receiving this because you authored the thread.Message ID: @.***>

vmx commented 7 months ago

I haven't used serde_transcode, but it sounds like it could be the case that you could end up with a map-like structure which isn't a BTreeMap. If you've some minimal example to reproduce the issue I can have a look.

vmx commented 7 months ago

Hmm are regular maps not supposed to be key sorted?

What are "regular maps"? Everything map-like must be key sorted in order to be valid DAG-CBOR.

goeo- commented 7 months ago

trying to prepare a minimal example as we speak, but this is what's being called https://github.com/ipld/serde_ipld_dagcbor/blob/master/src/ser.rs#L269, and never collect_map

goeo- commented 7 months ago

here's the minimal example:

fn main() {
    let data = r#"
        {
            "a": "1",
            "b": "2"
        }"#;
    let mut deserializer = serde_json::Deserializer::from_str(data);
    let writer = serde_ipld_dagcbor::ser::BufWriter::new(Vec::new());
    let mut serializer = serde_ipld_dagcbor::ser::Serializer::new(writer);
    serde_transcode::transcode(&mut deserializer, &mut serializer).unwrap();
    println!("{:?}", serializer.into_inner().into_inner());

    let data = r#"
        {
            "b": "2",
            "a": "1"
        }"#;
    let mut deserializer = serde_json::Deserializer::from_str(data);
    let writer = serde_ipld_dagcbor::ser::BufWriter::new(Vec::new());
    let mut serializer = serde_ipld_dagcbor::ser::Serializer::new(writer);
    serde_transcode::transcode(&mut deserializer, &mut serializer).unwrap();
    println!("{:?}", serializer.into_inner().into_inner());
}

(you have to patch serde_ipld_dagcbor to expose Serializer of course)

vmx commented 7 months ago

@goeo- it would be great if you could try whether https://github.com/ipld/serde_ipld_dagcbor/pull/27 works for you. I've added tests for your use-case, but you never know ;)

goeo- commented 7 months ago

This seems to work, but looks like I can't rely on serde_transcode after all, as cids in js/json stay as text, and I think this should be handled by the transcoder and not serde_ipld_dagcbor

Edit: can confirm that it works now that I've edited the transcoder

vmx commented 7 months ago

Edit: can confirm that it works now that I've edited the transcoder

That's cool to hear. I guess that means that we'd have an "ipld transcoder" that works with codecs support CIDs (similar to https://github.com/ipld/rust-ipld-extract-links).