Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Using de/serialize_with inside of an Option, Map, Vec #723

Open
sfackler opened this issue Jan 26, 2017 · 40 comments
Open

Using de/serialize_with inside of an Option, Map, Vec #723

sfackler opened this issue Jan 26, 2017 · 40 comments

Comments

@sfackler
Copy link
Contributor

These would be equivalent to Jackson's @Serialize(keysUsing=...) etc. Now that we have stateful deserialization, this can be implemented in a pretty straightforward way.

I believe we'd want to support map keys and values as well was seq and option values.

@dtolnay
Copy link
Member

dtolnay commented Apr 7, 2017

Ideally I would like to find an approach that composes better than keysUsing.

#[derive(Deserialize)]
struct S {
    #[serde(deserialize_with = "my_key")]
    key: K,

    #[serde(deserialize_with = "my_value")]
    value: V,

    #[serde(???)]
    opt_map: Option<Map<K, V>>,
}

@dtolnay dtolnay changed the title Add {deserialize,serialize}_{keys,values}_with support to serde-derive Using de/serialize_with inside of an Option, Map, Vec Apr 8, 2017
@dtolnay
Copy link
Member

dtolnay commented Apr 8, 2017

#576 has an approach based on a helper for generating ordinary deserialize_with functions, rather than using a slate of new attributes.

@dtolnay
Copy link
Member

dtolnay commented Apr 8, 2017

This could be neat:

#[derive(Deserialize)]
struct S {
    #[serde(deserialize_with = "my_key")]
    key: K,

    #[serde(deserialize_with = "my_value")]
    value: V,

    #[serde(deserialize_with = "my_opt_map")]
    opt_map: Option<Map<K, V>>,
}

fn my_map<'de, D>(deserializer: D) -> Result<Map<K, V>, D::Error>
    where D: Deserializer<'de>
{
    deserialize_map_with!(my_key, my_value)(deserializer)
}

fn my_opt_map<'de, D>(deserializer: D) -> Result<Option<Map<K, V>>, D::Error>
    where D: Deserializer<'de>
{
    deserialize_option_with!(my_map)(deserializer)
}

@dtolnay dtolnay removed this from the v1.0 milestone Apr 9, 2017
@dtolnay
Copy link
Member

dtolnay commented Apr 10, 2017

Another possible composable approach:

#[derive(Deserialize)]
struct S {
    #[serde(deserialize_with = "my_k")]
    k: K,

    #[serde(deserialize_with = "option!(my_k)")]
    opt: Option<K>,

    #[serde(deserialize_with = "option!(map!(my_k, my_v))")]
    opt_map: Option<Map<K, V>>,

    #[serde(deserialize_with = "map!(_, my_v)")]
    map: Map<u8, V>,
}

@dtolnay
Copy link
Member

dtolnay commented Apr 20, 2017

This needs to support all the wrapper types too: Rc, Arc, Cell, RefCell, Mutex, RwLock.

@daboross
Copy link
Contributor

daboross commented May 16, 2017

Would a syntax like this also want to support custom key/values within a custom map deserializer?

Like

#[derive(Deserialize)]
struct S {
    #[serde(deserialize_with = "my_k")]
    k: K,

    #[serde(deserialize_with = "option!(my_k)")]
    opt: Option<K>,

    #[serde(deserialize_with = "my_map")]
    map: Map<u8, u8>,

    #[serde(deserialize_with = "option!(map!(my_k, my_v))")]
    opt_map: Option<Map<K, V>>,

    #[serde(deserialize_with = "map!(_, my_v)")]
    map: Map<u8, V>,

    #[serde(deserialize_with = "map::<my_map>!(my_k, my_v)")]
    map: Map<K, V>,
}

or would this not be feasible at all with the current Deserializer architecture?

@dtolnay
Copy link
Member

dtolnay commented May 16, 2017

Yes, probably by means of a trait that is implemented for all types that support map!, a different one that is implemented for types that support option!, etc.

@daboross
Copy link
Contributor

daboross commented May 16, 2017

hm, ok. Would that mean the my_map deserializer would have to return a type which implements some trait then?

I mean deserialize_with is often used with types from other libraries, where implementing a trait on the type isn't possible - just trying to hash out the problem here.

For a syntax alternative, what would you think of something like "inner" attribute, like this?

#[derive(Deserialize)]
struct S {
    #[serde(deserialize_with = "my_k")]
    k: K,

    #[serde(inner(K, deserialize_with = "my_k"))]
    opt: Option<K>,

    #[serde(deserialize_with = "my_map")]
    map: Map<u8, u8>,

    #[serde(inner(K, deserialize_with = "my_k"))]
    #[serde(inner(V, deserialize_with = "my_v"))]
    opt_map: Option<Map<K, V>>,

    #[serde(inner(V, deserialize_with = "my_v"))]
    map: Map<u8, V>,

    #[serde(deserialize_with = "my_map")]
    #[serde(inner(K, deserialize_with = "my_k"))]
    #[serde(inner(V, deserialize_with = "my_v"))]
    map: Map<K, V>,
}

If this kind of syntax would be allowed in attributes, and if it relatively matched what we can feasibly make happen, it would also provide an intuitive way to include #914:

#[derive(Deserialize)]
struct S {
    #[serde(inner(Cow<'a, str>, borrow))]
    cow: Vec<Cow<'a, str>>,
}

Edit: again, thinking entirely " ideal situation" here, but could we have this attribute support literally all field attributes by having the Deserialize impl create a newtype for each #[inner] clause with the inner attributes?

If I understand the situation correctly, having it create a newtype would let any inner attributes apply to the newtypos single field, and the changes in deserialization could be fully conveyed statically with no cost.

Thoughts?

Or... any obvious contradictions which I completely overlooked which would invalidate this?

@daboross
Copy link
Contributor

Sorry, I think I was just on the complete wrong track there. My apologies for not researching how all of this actually works and thinking about it before commenting!

I think I can agree now that a trait is probably the best way to do this.

@HighCommander4
Copy link

I'm pretty new to Rust and Serde, but since you asked (in e.g. #999 and #1005) for people's thoughts on a design, here's how I would like this to work as a user:

mod remote {   // the remote crate
  struct Foo {    // the remote struct I'm using
    ...
  }
}
////////////////
mod my {   // my crate
  #[derive(Serialize, Deserialize)]
  struct FooDef {  // a struct with identical fields to Foo
    ...
  }

  // A macro invocation that emits some declarations that make
  // it so that, within this module, _every_ use of Foo is serialized
  // as if it had been annotated with #[serde(with = "FooDef")]
  serde_serialize_as!(Foo, FooDef);

  #[derive(Serialize, Deserialize)]
  struct MyStruct {
    field1: Foo,   // works, no additional annotation necessary
    field2: Option<Foo>,   // also works
    field3: HashMap<Foo, String>   // also works
  }
}

I don't know enough about Rust macros and Serde to say whether this is actually implementable.

@Keats
Copy link

Keats commented Feb 23, 2018

I guess no design has been decided yet? I have a struct that has a Option<toml::value::Datetime> but I'd like to store it as a String instead since I don't want to care about TOML once it's loaded.

fn from_toml_datetime<'de, D>(deserializer: D) -> StdResult<Option<String>, D::Error>
    where
        D: Deserializer<'de>,
{
    toml::value::Datetime::deserialize(deserializer)
        .map(|s| Some(s.to_string()))
}

was my attempt at #[serde(deserialize_with = "from_toml_datetime")]

The above was my intuition so I would vote for something like that, if it's doable.

Is there a way to do that at all currently?

@daboross
Copy link
Contributor

daboross commented Feb 23, 2018

@Keats that looks like it should work correctly. Is a field marked with #[serde(default, deserialize_with = "from_toml_datetime")] not working?

Edit: just realized you included the attribute. I would recommend also adding #[serde(default)] to handle the case where it's not there - this is implied regularly, but is not when there's a custom deserialize like this.

@Keats
Copy link

Keats commented Feb 24, 2018

I was missing the default, that did the trick. Thanks!

@daboross
Copy link
Contributor

daboross commented Mar 4, 2018

What does anyone think of including non-(de)serialize_with attributes in this issue?

I'm currently running into a situation where it would make sense to use a HashMap<i64, Cow<'a, [u8]>> with all inner Cow's borrowed. However, #[serde(borrow)] only affects the outermost type, just like (de)serialize_with.

@zbraniecki
Copy link

zbraniecki commented Apr 18, 2018

Is there any way to workaround it? I have a struct like:

struct Snippet {
    annotations: Vec<Annotation>
}

#[derive(Deserialize)]
#[serde(remote = "Snippet")]
struct SnippetDef {
  #[serde(with = "AnnotationDef")]
  annotations: Vec<Annotation>
}

and that of course doesn't work. How should I solve it until this is fixed?

@daboross
Copy link
Contributor

@zbraniecki

My understanding is that you'd work around it by making a method, roughly:

fn deserialize_annotation_vec<'de, D>(deserializer: D) -> Result<Vec<Annotation>, D::Error> {
    struct AnnotationVecVisitor;
    impl<'de> Visitor<'de> for AnnotationVecVisitor {
        type Value = Vec<Annotation>;
        fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { write!(f, "a list of annotations") }
        fn visit_seq<A: SeqAccess<'de>>(self, seq: A) -> Result<Vec<Annotation>, A::Error> {
            let mut vec = Vec::with_capacity(cmp::min(seq.size_hint().unwrap_or(0), 4096));
            while let Some(v) = seq.next_element()? {
                // assert type
                let v = <AnnotationDef as Into>::into(v);
                vec.push(v);
            }
            Ok(vec)
        }
    }

    deserializer.deserialize_seq(AnnotationDefVisitor)
}

Then you'd use #[serde(deserialize_with = "deserialize_annotation_vec"] on the field to use this implementation.

Sources:

@dtolnay
Copy link
Member

dtolnay commented Apr 18, 2018

I would implement it as:

use serde::{Deserialize, Deserializer};

#[derive(Deserialize)]
#[serde(remote = "Snippet")]
struct SnippetDef {
    #[serde(deserialize_with = "vec_annotation")]
    annotations: Vec<Annotation>,
}

fn vec_annotation<'de, D>(deserializer: D) -> Result<Vec<Annotation>, D::Error>
where
    D: Deserializer<'de>,
{
    #[derive(Deserialize)]
    struct Wrapper(#[serde(with = "AnnotationDef")] Annotation);

    let v = Vec::deserialize(deserializer)?;
    Ok(v.into_iter().map(|Wrapper(a)| a).collect())
}

@aleokdev
Copy link

Any progress on this? I've tried to use the serde_with crate, but i can't find any documentation explaining how to use it with remote types. I have a Vec of a remote type and I just can't figure out how to serialize it without doing it manually.

@jonasbb
Copy link
Contributor

jonasbb commented Nov 25, 2020

@alexdevteam I'd love to hear in more details what you tried with serde_with and what didn't work. Please post it here.

@colmanhumphrey
Copy link

Recently I tried to use @dtolnay's excellent string_or_struct function on a vector.

Building on David's code, let's assume we have instead:

#[derive(Deserialize, Serialize)]
pub struct ManyBuilds {
    some_fields_with_overall_info: i32, // etc
    // ideally we'd have #[serde(deserialize_with = "string_or_struct") or other notation
    builds: Vec<Build>
}
// see https://serde.rs/string-or-struct.html for Build's definition and traits

I tried @dtolnay's also-excellent wrap function from this thread, although I got generic and lifetime errors that I don't yet have the skills to resolve (I'll try again!). I also couldn't find how to do this with serde_with, but let me know if there's a clear choice @jonasbb and I'd be happy to try and/or make an issue.

My current solution is a newtype:

#[derive(Deserialize, Serialize)]
pub struct ManyBuildsInput {
    some_fields_with_overall_info: i32, // etc
    builds: Vec<BuildWrap>
}

#[derive(Deserialize, Serialize)]
struct BuildWrap(#[serde(deserialize_with = "string_or_struct") Build)

Then implement From/TryFrom etc to and from the ManyBuildsInput struct to ManyBuilds. Extra work, but survivable!

(by the way - @dtolnay thanks for the crate, serde is amazing!)

@jonasbb
Copy link
Contributor

jonasbb commented Jun 30, 2021

@colmanhumphrey I am happy for any feedback and if you need help using serde_with just open a new discussion. This should solve the problem:

    #[serde_as(as = "Vec<PickFirst<(_, DisplayFromStr)>>")]
    //                              ^ deserialize struct
    //                                 ^^^^^^^^^^^^^^ or FromStr
    builds: Vec<Build>

@colmanhumphrey
Copy link

@colmanhumphrey I am happy for any feedback and if you need help using serde_with just open a new discussion. This should solve the problem:

    #[serde_as(as = "Vec<PickFirst<(_, DisplayFromStr)>>")]
    //                              ^ deserialize struct
    //                                 ^^^^^^^^^^^^^^ or FromStr
    builds: Vec<Build>

Thanks @jonasbb - just created jonasbb/serde_with#333!

@vi
Copy link

vi commented Sep 17, 2021

Given serde already reached version 1, does this issue have a chance to be resolved by Serde itself (not by some external mean), preserving compatibility?

Maybe inability to use containers should be documented explicitly, ideally with some special error message that points users to workarounds?

@DanielJoyce
Copy link

Just bumped into this as well, surprised it's still not handled.

@chanced
Copy link

chanced commented Mar 24, 2023

Unless I'm missing something, this seems not only possible but fairly ergonomic? For example:

pub struct Jwk {
    #[serde(
        with = "b64::optional_seq_url_safe",
        skip_serializing_if = "Option::is_none",
        default
    )]
    pub x5c: Option<Vec<Vec<u8>>>,
}

pub(crate) mod b64 {
    pub(crate) mod optional_seq_url_safe {
        #[cfg(not(feature = "std"))]
        use alloc::{string::String, vec::Vec};
        use base64::{engine::general_purpose::URL_SAFE_NO_PAD, Engine as _};
        use serde::{self, ser::SerializeSeq, Deserialize, Deserializer, Serializer};

        pub fn serialize<S, V, T>(input: &Option<T>, serializer: S) -> Result<S::Ok, S::Error>
        where
            S: Serializer,
            T: AsRef<[V]>,
            V: AsRef<[u8]>,
        {
            if input.is_none() {
                return serializer.serialize_none();
            }
            let input = input.as_ref().unwrap().as_ref();
            let mut ser = serializer.serialize_seq(Some(input.len()))?;
            for item in input {
                ser.serialize_element(&URL_SAFE_NO_PAD.encode(item.as_ref()))?;
            }
            ser.end()
        }

        pub fn deserialize<'de, D, T>(deserializer: D) -> Result<T, D::Error>
        where
            D: Deserializer<'de>,
            T: From<Option<Vec<Vec<u8>>>>,
        {
            let data = Option::<Vec<String>>::deserialize(deserializer)?;
            let data = data
                .map(|data| {
                    data.into_iter()
                        .map(|data| {
                            URL_SAFE_NO_PAD
                                .decode(data)
                                .map_err(serde::de::Error::custom)
                        })
                        .collect::<Result<Vec<_>, _>>()
                })
                .transpose()?;
            Ok(data.into())
        }
    }
}

@daboross
Copy link
Contributor

daboross commented Mar 24, 2023

The main problem with that is just that you have to write a custom module for every different modification. This issue is asking for a generic way to turn some serialization module that serializes/deserializes X into one that works on Option<X>, and Vec<X>, and Option<Vec<T>>, and HashMap<String, T>, etc.. Ideally, it'd include any nesting. But if that's not possible, then at least custom code for each nesting should only have to be written once per nesting, not once per nesting per custom serialize/deserialize module.

Doing it manually works fine, if you know what's needed. But if you're writing a crate for general use, what modules do you provide?

I've done this in the past for internal usage too, usually having two modules, do_thing and do_thing_optional. It's just not general: this doesn't let me do my custom code when deserializing a Vec<Thing> or HashMap<,u8, Thing>. You'd need to write two more modules to cover those two use cases.

It's annoying to write N modules for N different use cases, but I mean sure it works if you're the only user. It'd be much more out of hand if you were trying to write a library, though.

For my use case for this, https://github.com/daboross/serde-tuple-vec-map/, I've experimented with another option: just provide a wrapper type in addition to the main module. Have someone do field: Option<serde_tuple_vec_map::Wrapper<Vec<(String, String)>>> to work around the fact that they can't do #[serde(with = "serde_tuple_vec_map")] in that case. It still has a big downside, though, of requiring them to insert a wrapper type whose only purpose is interacting with serde into their data structure, and deal with it throughout their codebase.

@murar8
Copy link

murar8 commented Feb 2, 2024

Just released a small crate to help work around the issue: https://crates.io/crates/serde_nested_with
It will generate the workaround module for you.

tesaguri added a commit to tesaguri/kitsune that referenced this issue Feb 24, 2024
tesaguri added a commit to tesaguri/kitsune that referenced this issue Feb 25, 2024
github-merge-queue bot pushed a commit to kitsune-soc/kitsune that referenced this issue Mar 7, 2024
* Add helpers to deserialise JSON-LD in a more robust way

* Add brief introduction to JSON-LD data model

* Fix deserialisation of `Option<_>` fields

Workaround for <serde-rs/serde#723>.

* Remove `AttributedToListEntry` type

* Minor cleanup

* Replace `&x[..]` with `x.as_str()`

Per review comment at:

<#492 (comment)>.

Co-Authored-By: Aumetra Weisman <aumetra@cryptolab.net>

---------

Co-authored-by: Aumetra Weisman <aumetra@cryptolab.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests