-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Description
FluentValidation version
11.2.2
ASP.NET version
.NET 6
Summary
It appears that ScalePrecisionValidator produces an error message which does not quite reflect what is actually being validated.
I have a decimal property in my model class, which I want to be of precision 10 and scale 2. So in theory a value like 123456789 should be fine, since by not providing any zeros after decimal point the total precision of this number is 9. And it seems that ScalePrecisionValidator calculates precision correctly, but it still gives a validation error and a message like this:
'{PropertyName}' must not be more than 10 digits in total, with allowance for 2 decimals. 9 digits and 0 decimals were found.
Which sounds like a nonsense. After a bit of digging the FluentValidation source code it turned out what is actually being validated by ScalePrecisionValidator:
public override bool IsValid(ValidationContext<T> context, decimal decimalValue) {
var scale = GetScale(decimalValue);
var precision = GetPrecision(decimalValue);
var actualIntegerDigits = precision - scale;
var expectedIntegerDigits = Precision - Scale;
if (scale > Scale || actualIntegerDigits > expectedIntegerDigits) {
context.MessageFormatter
.AppendArgument("ExpectedPrecision", Precision)
.AppendArgument("ExpectedScale", Scale)
.AppendArgument("Digits", actualIntegerDigits < 0 ? 0 : actualIntegerDigits)
.AppendArgument("ActualScale", scale);
return false;
}
return true;
}So it does not actually validate precision at all. What is being validated is that the integer part of the value must not overflow expected amount of digits, i.e. Precision - Scale amount of digits, which in my case was 8 digits. And now it makes sense why validation does not pass, because indeed my number has 9 digits in the integer part. But what is being communicated back to the user as validation error message and what is being validated are two different things. Without knowing this implementational detail this might be quite hard to the user to make out why validation does not pass.
This issue would actually relate to the abandoned issue #1200, but the proposal stated there would not resolve the problem I think, since in this case, for example, giving the total precision and giving amount of integer digits would result in the same number 9. So I would say that it might be better to communicate 'ExpectedIntegerDigits' as something that was expected from a decimal value to have, and what is indeed validated. And the error message would sound something like:
'{PropertyName}' must have at most 8 integer digits and 2 decimals. 9 integer digits and 0 decimals were found.
In this manner the error message would make sense.
But also as stated in #1200, it is true that such a validation logic does not correspond with what is actually defined by scale and precision. But such a thing is discutable, and the error message problem is on the other hand more specific.
Steps to Reproduce
This can be quite easily reproduced. Assume we have a class A with a decimal property Property:
public class A
{
public decimal Property { get; set; }
}And we want this property to be of precision 10 and scale 2:
public class AValidator : AbstractValidator<A>
{
public AValidator()
{
RuleFor(x => x.Property).ScalePrecision(2, 10);
}
}So we try to give it a number like 123456789 and to execute validation on it:
public class Program
{
public static void Main(string[] args)
{
var a = new A()
{
Property = 123456789,
};
var validationResult = new AValidator().Validate(a);
if (!validationResult.IsValid())
{
foreach (var error in validationResult.Errors)
{
Console.WriteLine(error.ErrorMessage);
}
}
else
{
Console.WriteLine("A is ok!");
}
}
}And instead of not having any validation error at all or at least having an error message which would seem reasonable the result would be:
'Property' must not be more than 10 digits in total, with allowance for 2 decimals. 9 digits and 0 decimals were found.