serde-rs / serde

Serialization framework for Rust
https://serde.rs/
Apache License 2.0
9.21k stars 778 forks source link

Support a #[serde(validate = "some_function")] attribute on fields #939

Open sfackler opened 7 years ago

sfackler commented 7 years ago

I occasionally run into fields in a serde-deserializable type that require some extra validation after deserialization, for example "a u32 >= 2". The way to do this now is to make a custom deserialize function that deserializes and then does the check:

#[derive(Deserialize)]
struct Thing {
    #[serde(deserialize_with = "validate_foo")]
    foo: u32,
}

fn validate_foo<'de, D>(d: D) -> Result<u32, D::Error>
    where D: de::Deserializer<'de>
{

    let value = u32::deserialize(d)?;

    if value < 2 {
        return Err(de::Error::invalid_value(de::Unexpected::Unsigned(value as u64),
                                            &"a value at least 2"));
    }

    Ok(value)
}

However, it'd be nice if it were possible to specifically tell serde to use the standard deserializer but apply a validation function after the field is deserialized:

#[derive(Deserialize)]
struct Thing {
    #[serde(validate = "validate_foo")]
    foo: u32,
}

fn validate_foo<E>(v: &u32) -> Result<(), E>
    where E: de::Error
{
    if value < 2 {
        Err(de::Error::invalid_value(de::Unexpected::Unsigned(value as u64),
                                     &"a value at least 2"));
    } else {
        Ok(())
    }
}
Keats commented 7 years ago

That's very similar to one of my crate (https://github.com/Keats/validator) with a wip new version at https://github.com/Keats/validator/pull/27

lucab commented 7 years ago

I guess this is a duplicate of https://github.com/serde-rs/serde/issues/642.

dtolnay commented 7 years ago

I don't think this is a duplicated of #642. This is for a field attribute and that one is for a container attribute. They are useful in different situations.

TedDriggs commented 7 years ago

@sfackler, how would you feel about this:

#[derive(Deserialize)]
struct Thing {
    #[serde(and_then = "validate_foo")]
    foo: u32,
}

fn validate_foo<E>(v: u32) -> Result<u32, E>
    where E: de::Error
{
    if value < 2 {
        Err(de::Error::invalid_value(de::Unexpected::Unsigned(value as u64),
                                     &"a value at least 2"));
    } else {
        Ok(value)
    }
}

That would enable both validation and post-deserialize transformation, which is something I've found myself reaching for in the past.

Mingun commented 4 years ago

I think this one is superseded by #1550 and can be closed in preference to it

avkonst commented 3 years ago

I think this issue should be closed as it seems there is a solution with and_then. Otherwise, it makes the impression it requires work....

TedDriggs commented 3 years ago

@avkonst it appears that #1858 was closed without being merged, meaning this particular issue probably will need to instead reference a documentation update.

@dtolnay your logic for individual fields in this comment makes sense; the place where I've found myself reaching for and_then is with complex objects such as the OASIS CACAO specification. That has requirements such as:

  1. If both valid_from and valid_until are specified, then valid_from MUST be less than valid_until
  2. All variables mentioned in a workflow step MUST be defined either in the playbook's variables or in the step's variables

If I'm also providing some other avenue for making these objects, then I likely already have a function with the signature validate(Playbook) -> Result<Playbook, AnErrSerdeCanWorkWith> in existence, and I'd like to have serde run the struct through that.

An alternative would be a custom Deserialize impl on the Playbook struct, like this:

pub struct Playbook {
    // fields
}

impl<'de> Deserialize<'de> for Playbook {
    fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
    where
        D: serde::Deserializer<'de>,
    {
        #[derive(Deserialize)]
        struct UncheckedPlaybook {
            // fields...
        }

        impl TryFrom<UncheckedPlaybook> for Playbook {
            type Error = AnErrorThatImplsDisplay;

            fn try_from(_: UncheckedPlaybook) -> Result<Self, Self::Error> {
                // elided...
            }
        }

        UncheckedPlaybook::deserialize(deserializer)?.try_into().map_err(D::Error::custom)
    }
}

That works, but when the struct in question has a lot of fields that's code drift waiting to happen. I've considered making a crate to solve this that would allow the generation of a replica struct with different serde attributes, but that seems like overkill for the common case.

AsafFisher commented 3 years ago

deserialize_with Can be used for validation, please close this.

EqualMa commented 3 years ago

deserialize_with is not easy for me to use. But I find try_from can be easily used to validate types and fields.

#[derive(Deserialize)]
#[serde(try_from = "String")] // Tell serde to deserialize data into a String and then try to convert it into Email
pub struct Email(String);

impl TryFrom<String> for Email {
    type Error = String;

    fn try_from(value: String) -> Result<Self, Self::Error> {
        if validate_email(&value) {
            Ok(Self(value))
        } else {
            Err(format!("Invalid email {}", value))
        }
    }
}

Then Email can be deserialized from a string and it will be validated.

You can also use it as the type of a field. It will be validated when the struct is deserialized.

#[derive(Deserialize)]
pub struct User {
    name: String,
    email: Email,
}

For full working code, checkout this repo. I also wrote a a detailed guide.

schneems commented 3 years ago

@EqualMa thank you so much for that comment and the full write-up. I really like that approach. I'm a new-ish Rust programmer (decade in Ruby). The approach seems to work well.

I'm curious what it would take to be able to close out several of these requests for validation issues. I'm wondering would adding official documentation on that approach be enough for now?

I think in the future maybe this approach could be simplified/automated even more, and could be wrapped into some more specific "validation" feature. For example, maybe automating the generation of the "shadow" unchecked struct and the core "try from" logic. But the programmer has to implement the Ok/Err logic either way. All that considered TBH it's not a ton of boilerplate needed to use this approach.

What do you all think? Document this today to be able to close out some of these feature issues? If someone wants to further add some automation in the future PRs welcome?

rapiz1 commented 2 years ago

@dtolnay 's comment https://github.com/serde-rs/serde/pull/1858#pullrequestreview-575089757 provides a quick example of validation using deserialize_with. I feel the need to link it here to help those who's looking for a solution.

Maybe this should be summed up and added to the documentation.

schneems commented 2 years ago

Maybe this should be summed up and added to the documentation.

Yes, I agree. I feel there are several issues and prs that we could close out as "good enough" with some existing solutions documented.

CobaltCause commented 2 years ago

I just realized that one can use const generics for integer validators; a bit much to declare but pretty nice to use.

ku1ik commented 2 years ago

I feel like conflating parsing (aka deserialization, what serde was built for) with value validation is not the right way to go. I know it's tempting to combine the two operations, it'd be convenient, but convenience invites complexity. Validation could easily be done as a second step, on the serde deserialized data structure. @Keats's validator crate seems nice (my understanding is you call .validate() on the struct after serde successfully deserialized).

ssokolow commented 2 years ago

Personally, I think that what people are asking for as "validation" is more a subset of parsing.

Parsing a sequence of UTF-8 bytes into an XML DOM doesn't become "validation" just because < or > or & are only valid in certain positions, and an argument can be made that one shouldn't need to newtype everything just to enforce that an integer named week_of_year only has a certain valid range.

...that said, my approach generally is to newtype everything since that both lets me push "validation" into Serde and allows the compiler to prevent conflation of values. Parse, don't validate.

CobaltCause commented 2 years ago

Parse, don't validate is probably one of my top 3 favorite blog posts of all time

Another reason I favor newtypes over a second "validation" step is that without newtypes you can write this code:

let x = Stuff {
    between_1_and_3: 200,
};

which is clearly wrong. But with my example above, you can do either of:

between_1_and_3: BoundedUsize::clamp(200),
between_1_and_3: BoundedUsize::new(200).expect("out of bounds"),

And you could feasibly write a macro like this that would cause a compile error instead of a runtime one:

between_1_and_3: bounded_usize!(200),

I think the common name for this pattern is "correct by construction", which seems more in line with the way the rest of Rust works.

kanarus commented 3 months ago

"Parse, don't validate" is ideal in many situations, but if you need serde with validation, check out SerdeV ( https://github.com/ohkami-rs/serdev ), essentially serde with #[serde(validate = "some_function")] container attribute. This allows you to validate automatically in deserialization with no boilerplate.