serde icon indicating copy to clipboard operation
serde copied to clipboard

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

Open sfackler opened this issue 7 years ago • 16 comments

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(())
    }
}

sfackler avatar May 19 '17 20:05 sfackler

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

Keats avatar May 28 '17 15:05 Keats

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

lucab avatar May 30 '17 11:05 lucab

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.

dtolnay avatar May 30 '17 14:05 dtolnay

@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.

TedDriggs avatar May 31 '17 15:05 TedDriggs

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

Mingun avatar Oct 24 '20 07:10 Mingun

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....

avkonst avatar Apr 19 '21 23:04 avkonst

@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.

TedDriggs avatar Apr 27 '21 13:04 TedDriggs

deserialize_with Can be used for validation, please close this.

AsafFisher avatar May 18 '21 23:05 AsafFisher

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.

EqualMa avatar Oct 10 '21 16:10 EqualMa

@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?

schneems avatar Oct 20 '21 18:10 schneems

@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.

rapiz1 avatar Dec 06 '21 13:12 rapiz1

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.

schneems avatar Dec 07 '21 19:12 schneems

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

CobaltCause avatar Jun 17 '22 23:06 CobaltCause

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).

ku1ik avatar Aug 03 '22 11:08 ku1ik

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.

ssokolow avatar Aug 03 '22 12:08 ssokolow

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.

CobaltCause avatar Aug 03 '22 15:08 CobaltCause