-
-
Notifications
You must be signed in to change notification settings - Fork 788
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
Add finalizer attribute hook to validate a deserialized structure #642
Comments
This seems relevant: https://github.com/Keats/validator. Let's make sure to support this library in a nice way. |
My hacky workaround is to
It's a dumb hack but at least it allows me to avoid rewriting all the deser code manually. |
Another use-case from irc by @alteous
|
I believe I could also use this for an enum which is either "unknown" or some value fitting a specific pattern, but both are strings. |
I have the same usecase as in #626. I want to load shapes, and I also want to store some precomputed attributes on them that are derived from the other fields (for example I don't want to serialize or deserialize these fields, but I want them to be present in the finished structs. The shapes could be deeply nested in another structure that is loaded, so it's very messy to try to go down the structures in a mutable fashion and set it on the structs manually. |
Perhaps a silly question, but why an attribute instead of a trait? I pictured:
(And maybe |
I've been working on something related to this for
should be split across two different types, and that the sidecar type could be generated through a combination of macros, i.e. deriving a Builder which in turn derives Deserialize. In that world, field-level validations would be handled through types, rather than the attributes in the validator project. I've run into a few hurdles so far:
|
Cross-referencing Keats/validator#22. It looks like one of the main concerns there is how to bubble up structured/detailed errors when validation/finalization fails. If anybody has usecases/opinions, it would be nice to feed them back there. |
There seem to be two situations for this:
From a naming perspective, these seem exactly like @lucab I've been going back and forth on the errors front. Having that function return a serde Error is really useful for composition with this feature; maybe users can handle that conversion in their own code? |
Any progress on this lately? |
It would be nice to have a hook like the one mentioned by @scottmcm. With validator, I could do something like #[derive(Debug, Validate, Deserialize)]
#[serde(Finalize)]
struct SignupData {
#[validate(email)]
mail: String,
#[validate(url)]
site: String,
#[validate(length(min = "1"), custom = "validate_unique_username")]
#[serde(rename = "firstName")]
first_name: String,
#[validate(range(min = "18", max = "20"))]
age: u32,
}
// This
impl SerdeFinalize for SignupData {
fn finalize<ValidationErrors>(self, deserialization_errors: Option<HashMap<Cow<String>, serde::de::Error>>) -> Result<Self, ValidationErrors> {
if let Some(de_errors) = deserialization_errors {
// format errors and return without doing any validation?
}
// the current Validate::validate fn would be impl here instead
}
}
// And use it later
let data: SignupData = serde_json::from_str(data)?; This way it's a one step action for users. The part I'm not sure about is if a user wants to hook into the serde error messages to add i18n for example or to have only one kind of error. I believe https://docs.serde.rs/serde/de/trait.Error.html would be enough for that but those errors will need to be passed to whatever |
We are brainstorming some ways to generalize this in #1118. |
Note that the #[derive(Serialize, Deserialize)]
#[serde(try_from="usize")]
struct GreaterThan10(usize);
impl TryFrom<usize> for GreaterThan10 {
type Error = &'static str;
fn try_from(value: usize) -> Result<Self, Self::Error> {
if value > 10 {
Ok(GreaterThan10(value))
} else {
Err("value not greater than 10")
}
}
} |
@bstrie's workaround is pretty good. For custom structs, you can do make a shadow type, implement deserialize on it, and do validations within the TryFrom implementation. use serde::Deserialize;
use serde_json;
// The target is to not allow deserialization if option1 & option2 are none
#[derive(Deserialize, Debug)]
#[serde(try_from = "MyTypeShadow")]
pub struct MyType {
option1: Option<usize>,
option2: Option<usize>,
}
// The shadow type only has to implement Deserialize
#[derive(Deserialize)]
struct MyTypeShadow {
option1: Option<usize>,
option2: Option<usize>,
}
pub struct MyTypeValidationError;
// The error type has to implement Display
impl std::fmt::Display for MyTypeValidationError {
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
write!(formatter, "option1 and option2 cannot be null")
}
}
impl std::convert::TryFrom<MyTypeShadow> for MyType {
type Error = MyTypeValidationError;
fn try_from(shadow: MyTypeShadow) -> Result<Self, Self::Error> {
let MyTypeShadow { option1, option2 } = shadow;
if option1.is_none() && option2.is_none() {
return Err(MyTypeValidationError);
}
// Any other validations
Ok(MyType { option1, option2 })
}
}
fn main() {
// This will return an Err
println!("{:?}", serde_json::from_str::<MyType>(r##"{}"##));
// This will work
println!(
"{:?}",
serde_json::from_str::<MyType>(r##"{"option1": 20}"##)
);
} |
Would a PR implementing the approach in the OP (#642 (comment)) be accepted (a |
A finalizer fn for deserialization is also useful for when you were forced to do a |
Another use case: Discord API libraries serenity and twilight. Some JSON payloads omit important data from nested objects because it can be inferred from top-level fields. But when deserializing into a Rust type, we want to include that important data in the nested object itself. Serenity's current workaround is a manual Deserialize impl that deserializes into a serde_json::Value, patches that, and then continues to deserialize into the target type (1). In other places, it manually parses from the patched serde_json::Value into the target type directly (2, 3) Twilight's current workaround is a full blown 710LOC manual Deserialize impl (1) |
Just skimmed over this ticket, and IIUC, there's no built-in support for this in The Because this pattern seems useful and somewhat common, I just submitted serde-rs/serde-rs.github.io#148 to add it to https://serde.rs . |
- Introduce Serializable trait, which I think will solve the problem of ephemeral struct fields getting reconstituted when we deserialize a Serde struct. (Note serde-rs/serde#642, which could eventually supersede this stuff.) - Added a few more hardcoded [MidiNote]s. - More [MusicalTime] math ops.
In servo/rust-url#252, @Manishearth is writing a serde deserializer for urls since parsing a string can be expensive. However, to do this safely from untrusted sources, they need to validate that the data is semantically correct. We unfortunately don't have an easy way of doing this without writing a custom deserializer. It'd be pretty simple to do as an attribute, however. Here's my idea:
The text was updated successfully, but these errors were encountered: