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

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

Open
sfackler opened this issue May 19, 2017 · 17 comments
Open

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

sfackler opened this issue May 19, 2017 · 17 comments

Comments

@sfackler
Copy link
Contributor

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
Copy link

Keats commented May 28, 2017

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

@lucab
Copy link

lucab commented May 30, 2017

I guess this is a duplicate of #642.

@dtolnay
Copy link
Member

dtolnay commented May 30, 2017

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
Copy link
Contributor

@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

This comment has been minimized.

@avkonst
Copy link

avkonst commented Apr 19, 2021

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
Copy link
Contributor

@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
Copy link

AsafFisher commented May 18, 2021

deserialize_with Can be used for validation, please close this.

@EqualMa
Copy link

EqualMa commented Oct 10, 2021

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 added a commit to heroku/libcnb.rs that referenced this issue Oct 20, 2021
When a `Stack` struct has a `StackId` of `*` then it should have no mixins.

This is validated by a "shadow" struct deserializing working from https://dev.to/equalma/validate-fields-and-types-in-serde-with-tryfrom-c2n.

Based on the comment from serde-rs/serde#939 (comment)

In my own words it works like this: We create a duplicate struct and use serde's normal `derive(Deserialize)` on it. Then we specify that the `Stack` struct should be converted using the duplicate struct with `#[serde(try_from = "StackUnchecked")]. Finally we implement `try_from` and include our special logic that checks to see we don't have a `*` stack id AND any mixins.

I also hard wrapped a few comments and put backpacks around special characters in the error output to make them clearer

Close #104
@schneems
Copy link

@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 added a commit to heroku/libcnb.rs that referenced this issue Oct 26, 2021
## Deserializing error background

When we deserialize TOML into structs there are requirements from the spec. For instance in https://github.com/Malax/libcnb.rs/pull/109 we see where this is invalid:

```
[[stacks]]
id = "*"
mixins = ["yolo"]
```

Because a stack cannot have an id of `*` and mixins. We need a consistent place to enforce this logic and rules and the most straightforward approach has been to check that the data at deserialize time.

## Existing approach

This deserialize validation behavior is already happening in the buildpack. For example, the `Deserialize` trait is manually implemented on `BuildpackAPI` here https://github.com/Malax/libcnb.rs/blob/2ca9fff2d449889ae0b9893326b068e43f370238/src/data/buildpack.rs#L86-L151.

## This approach

As part of the work with #109 it was found that serde has `try_from` that can be used to simplify this logic. Instead of having to manually implement all of Deserialize, the technique is to:

- Introduce an "unchecked" struct that holds the raw data before validation
- Implement a conversion between the "unchecked" struct and the "real" struct using TryFrom
- Implement the validation and error checking logic inside of TryFrom
- Set `#[serde(try_from=` on the "real" struct

A much longer example is demonstrated in this blog post https://dev.to/equalma/validate-fields-and-types-in-serde-with-tryfrom-c2n as pulled from serde-rs/serde#939 (comment)

Close #110
@rapiz1
Copy link

rapiz1 commented Dec 6, 2021

@dtolnay 's comment #1858 (review) 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
Copy link

schneems commented Dec 7, 2021

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
Copy link

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

@ku1ik
Copy link

ku1ik commented Aug 3, 2022

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
Copy link

ssokolow commented Aug 3, 2022

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
Copy link

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
Copy link

kanarus commented Aug 30, 2024

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

@oli-obk oli-obk added the docs label Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests