yassun7010 / serde_valid

JSON Schema based validation tool using serde.
MIT License
45 stars 10 forks source link

Custom validator that can return multiple errors #70

Closed Jacob-32587 closed 2 months ago

Jacob-32587 commented 2 months ago

What

For a property using a custom validator I want to return a list of validation errors.

Why

Currently I'm calling a function that can return many errors. The Validate macro seems to restrict custom functions to returning a singular error.

What I've tried

main.rs

use serde_json::json;
use serde_valid::{validation, Validate};
use zxcvbn::zxcvbn;

#[derive(Debug, serde::Deserialize, serde::Serialize, Validate)]
pub struct User {
    user_id: i64,
    username: String,
    user_email: String,
    #[validate(custom(|v| validate_password(v, &self.username, &self.user_email)))]
    password: String,
}

pub fn validate_password(password: &str, username: &str, user_email: &str) -> Result<(), validation::Error> {
    let res = zxcvbn(password, &[username, user_email]);
    match res.feedback() {
        Some(val) if res.score() <= zxcvbn::Score::Two => Err(validation::Error::Custom(json!(val).to_string())),
        _ => Ok(())
    }
}

impl User {
    pub fn new(user_id: i64, user_name: String, user_email: String, password: String) -> Self {
        Self { user_id, username: user_name, user_email, password,}
    }
}

fn main() {
    let user1 = User::new(0, String::from("Test User"), String::from("test@gmail.com"), String::from("123@testuser"));
    match user1.validate() {
        Ok(_val) => (),
        Err(err) => {
            println!("{:#}", err);
        },
    }
}

cargo.toml

[package]
name = "password_test"
version = "0.1.0"
edition = "2021"

[dependencies]
zxcvbn = { version = "3.0.1", features = ["ser"] }
serde_valid = { version = "0.22.0" }
serde = { version = "1.0.203" }
serde_json = { version = "1.0.117" }

The above example is where I left off, I attempted to serialize an object using serde_json and set it as the Custom variants value. Of course this did not work since the library has no way of knowing that my string is JSON and just serializes it as a string.

If there is a way of doing what I want let me know, otherwise would it be a big ask to allow a custom validator to return a Vec<validation::Error> (or something similar)?

(Thank you for any help in advance, this is a really cool validation library and I can't wait to use it in my personal project)

yassun7010 commented 2 months ago

The current system does not solve this.

The basic idea of this tool is to return one error per validation.

Returning multiple errors affects the basic philosophy, making it difficult to change it.

However, reading this Issue, it appears that you want to return a object error, not error message array.

See this draft PR test case.

Would this solution meet your desires?

yassun7010 commented 2 months ago

Your sample can be resolved as follows:

use serde_valid::{validation, Validate};
use zxcvbn::zxcvbn;

#[derive(Debug, serde::Deserialize, serde::Serialize, Validate)]
pub struct User {
    user_id: i64,
    username: String,
    user_email: String,
    #[validate(custom(|v| validate_password(v, &self.username, &self.user_email)))]
    password: String,
}

pub fn validate_password(
    password: &str,
    username: &str,
    user_email: &str,
) -> Result<(), validation::Error> {
    let res = zxcvbn(password, &[username, user_email]);
    match res.feedback() {
        Some(val) if res.score() <= zxcvbn::Score::Two => Err(validation::Error::CustomJson(
            serde_json::to_value(val).unwrap(),
        )),
        _ => Ok(()),
    }
}

impl User {
    pub fn new(user_id: i64, user_name: String, user_email: String, password: String) -> Self {
        Self {
            user_id,
            username: user_name,
            user_email,
            password,
        }
    }
}

fn main() {
    let user1 = User::new(
        0,
        String::from("Test User"),
        String::from("test@gmail.com"),
        String::from("pass"),
    );
    match user1.validate() {
        Ok(_val) => println!("User is valid."),
        Err(err) => {
            println!("{:#}", err);
        }
    }
}
yassun7010 commented 2 months ago

However, the type returned by this library is unclear. (I just noticed.)

For example, on the front side, a control statement would be needed to determine whether the error message is string or object.

The solution for clarity is to create custom error messages that are human readable.

pub fn validate_password(
    password: &str,
    username: &str,
    user_email: &str,
) -> Result<(), validation::Error> {
    let res = zxcvbn(password, &[username, user_email]);

    match res.feedback() {
        Some(val) if res.score() <= zxcvbn::Score::Two => {
            let message = val
                .warning()
                .map_or_else(|| "".to_string(), |warning| warning.to_string() + " ")
                + &val
                    .suggestions()
                    .iter()
                    .map(ToString::to_string)
                    .collect::<Vec<_>>()
                    .join(",");

            Err(validation::Error::Custom(message))
        }
        _ => Ok(()),
    }
}

Result:

{"errors":[],"properties":{"password":{"errors":["This is a top-100 common password. Add another word or two. Uncommon words are better."]}}}
Jacob-32587 commented 2 months ago

The CustomJson variant would work better for what I need. I'm using the below function as a place holder (similar to what you wrote). The issue with this is that Vue front-end code that uses my error messages is expecting an array of error messages to display everything properly. With the CustomJson method I could at least display the error messages without needing to split by a delimiter (such as \n). This means the Vue code would need to handle the type being a string | string [] type, which is much better than the current alternative.

pub fn validate_password(
    password: &str,
    username: &str,
    user_email: &str,
) -> Result<(), validation::Error> {
    let res = zxcvbn(password, &[username, user_email]);
    match res.feedback() {
        Some(val) => {
            let sug = val
                .suggestions()
                .iter()
                .map(|x| x.to_string())
                .chain(val.warning().iter().map(|x| x.to_string()))
                .collect::<Vec<String>>()
                .join("\n");
            Err(validation::Error::Custom(sug))
        }
        None => Ok(()),
    }
}
yassun7010 commented 2 months ago

This problem is that the result of the validation cannot be converted to a string easily.

If you want to return the array of each suggestion messages,this crate does not provide that feature.

This crate has fluent support for i18n, but how do I specify the fluent format if validation result returns the array of messages? I do not have a clear answer :(

Jacob-32587 commented 2 months ago

After reading up on fluent and i18n I am still a bit confused, why would a return type like Result<(), Vec<validation::Error>> break this standard? From what I understand (as far as the validation library is concerned) it is just a standard for how the final JSON should look, right? Would the CustomJson variant also go against the fluent specs? (Because this is still a much better option than splitting by \n characters in the front-end)

yassun7010 commented 2 months ago

it is just a standard for how the final JSON should look, right?

Yes. But returning string | Json instead of string is almost the same as string | any, which seems to throw away the type hint of front.

yassun7010 commented 2 months ago

Just now I came up with a solution to return multiple errors !!

https://github.com/yassun7010/serde_valid/pull/72

However, the release may be delayed due to another feature undergoing modification (implementation of warnings for old writing styles).

yassun7010 commented 2 months ago

It's close.

yassun7010 commented 2 months ago

@Jacob-32587

I released v0.23.0

Jacob-32587 commented 2 months ago

Sorry for the late response, thank you so much! This will help me a lot