-
-
Notifications
You must be signed in to change notification settings - Fork 777
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
Comments
That's very similar to one of my crate (https://github.com/Keats/validator) with a wip new version at Keats/validator#27 |
I guess this is a duplicate of #642. |
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. |
@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. |
This comment has been minimized.
This comment has been minimized.
I think this issue should be closed as it seems there is a solution with |
@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
If I'm also providing some other avenue for making these objects, then I likely already have a function with the signature An alternative would be a custom 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 |
|
#[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 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. |
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
@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? |
## 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
@dtolnay 's comment #1858 (review) provides a quick example of validation using 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. |
I just realized that one can use const generics for integer validators; a bit much to declare but pretty nice to use. |
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 |
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 ...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. |
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:
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. |
"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. |
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:
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:
The text was updated successfully, but these errors were encountered: