bartoszlenar/Validot

Validated value in the error message

bartoszlenar opened this issue · 3 comments

Feature description

  • To have an option to include the validated value in the error message.

Feature in action

m => m.EqualTo(666).WithMessage("The value {_model} is not equal to 666");
validator.Validate(555).ToString();
// The value 555 is not equal to 666

Feature details

  • That would be certainly another layer of creating the messages, {_model} placeholder would be in the cache and replaced with the actual value just during the validition.
  • ValidationContext would perform the operation during the validation.

Questions

That's technically, possible, but why implement this?
Is this really needed?

The fact that it's possible doesn't mean it's easy. And would certainly affect the perfromance.

If you do decide to go ahead with this, here's some things I've learned from doing this in FV:

  • If all you need is simple placeholders then the perfomance impact isn't really an issue (especially now with spans).
  • If you need placeholders with format info (eg like "{Value:c2}") (which FV does) then doing this with regex is significantly slower than using the FormatWith library https://github.com/crozone/FormatWith
  • Another option is not to use custom placeholders like this, and just allow regular string interpretation inside a callback, eg WithMessage(tokens => $"The value {tokens.Value} is not equal to 666") , which is tokenized at compile-time rather than at runtime (with hindsight, I wish I only supported this option in FV, and not the runtime placeholders, but string interpolation didn't exist when FV was started!)

@JeremySkinner, I'm still wondering whether I should prioritize this one high. It looks like a great feature, but how often in real life users insert full content of the validated value into the error message?

I see several potential risks here:

  • What if the content is corrupted, e.g., too large? Like a MiB-size string? Model validation stands as one of the front layers of incoming request verification. Isn't that a bit hazardous? Including the entire value to the report? A value that is always invalid, and technically, you don't control what it is? On the other hand - even if it's risky, the freedom to decide whether you want to do this is undoubtedly a nice thing.
  • Formatting? Too many types to handle. And what about custom classes? Calling ToString and trying to pass the args somehow? It does look like a challenging task. FormatWith looks excellent, but I want Validot to remain independent from third-party libs. Exactly as FluentValidation.

One thing that makes it a tough decision for me is that Validot's performance comes from the aggressive caching strategy. All error messages (in all languages, all the paths, etc.) are prepared upfront and delivered to the end report based on the validation logic. Inserting validated values into the error messages means creating them during each validation process. Thus, it instantly kills the main selling point that Validot has - being fast and memory efficient.

Also, relying so massively on caching makes the WithMessage(tokens => $"The value {tokens.Value} is not equal to 666") virtually impossible, as WithMessage evaluation and message constructing takes place during Validator initialization, before any real validation. It needs to be another layer or some trick that I need to came up with.

I'm torn apart really when it comes to this idea. I think I'll postpone it till I have a more extensive user base, and people start upvoting this thread.

FYI @JeremySkinner ultimately, I'm closing this down. The reasons as basically here in this thread for 2 years now. So far I haven't found justification for messing up so badly with the performance.

The only way I see it possible is at the very end, maybe in a form of a helper method to IValidationResult that would somehow correlate the value, the path and the message. Something more like this

User.Name ("elizabeth"): Must start with a capital letter

Where elizabeth is extracted from model and injected into the final printing.

But that's another story, nor another ticket. This one I'm closing down.