Keats/validator

ValidationError - Dynamic code and message

Opened this issue · 6 comments

garytaylor commented

Currently, ValidationError expects nothing but static string references for code, message and the key for the params. This works well in the majority of cases where we know at compile time which field we are validating.

However, I have a use case where I am validating random data (from serde json) against a soft schema stored in the database. Therefore, the fields to validate are not known at compile time, therefore static string references cannot be used.

https://github.com/Keats/validator/blob/master/validator/src/types.rs

Im fairly new to rust so I am not 100% sure if there is something I can do in my app to work around this or if this use case needs a change to this library ?

If the latter, is the change possible ? I am happy to do a PR for it

Keats commented

They are already using Cow, are you sure it doesn't fit?

IniterWorker commented

Please @garytaylor could you provide an example to describe the issue?
Thanks,

SuperFluffy commented

@IniterWorker the definition of ValidationError is:

#[derive(Debug, PartialEq, Clone, Serialize, Deserialize)]
pub struct ValidationError {
    pub code: Cow<'static, str>,
    pub message: Option<Cow<'static, str>>,
    pub params: HashMap<Cow<'static, str>, Value>,
}

What's probably confusing is that ValidationError::new is the only constructor, taking a &'static str. The trick is to use the public fields directly. That's not very conventional, but works:

let err = ValidationError {
    code: Cow::Owned(format!("a dynamic {}", "code")),
    message: Some(Cow::Owned(format!("a generated message with a number {}", 1234u64))),
    params: HashMap::new(),
};
IniterWorker commented

The ValidationError::new comes from 6 years old legacy code. So, the last refactoring 4y old of this section changed the fields' visibility and types. So, refactoring &'static str into a Cow will be an API-breaking change.

  • Continue to work with pub fields.
  • Create a new constructor: fn new_cow.

Thank you for the highlight @SuperFluffy,

SuperFluffy commented

Given that validator is pre-1.0, I think a breaking API change wouldn't be terrible, to be honest.

  • 👍1
Keats commented

It would be fine to change the constructor to take a Cow, it has likely been missed the last time

  • 👍1