Map a comparison rule only to a numeric property

The contribution context carries the property now, so a rule on another type,
the ordinal comparison of two strings for example, no longer publishes a
numeric bound. Two bounds are compared as decimals, which are exact for every
integral type, and only fall back to double for the magnitudes decimal can
not hold.

docs/en/framework/fundamentals/fluent-validation.md

@@ -93,7 +93,7 @@ The following rules are mapped:
 | FluentValidation rule | API definition |
 |---|---|
 | `NotNull()`, `NotEmpty()` | `IsRequired` |
-| `Length(min, max)`, `MinimumLength(min)`, `MaximumLength(max)` | `MinLength`, `MaxLength` |
+| `Length(min, max)`, `MinimumLength(min)`, `MaximumLength(max)` | `MinLength`, `MaxLength` (a zero bound is left out, see below) |
 | `Matches(...)` | `Regex` |
 | `GreaterThanOrEqualTo(...)`, `GreaterThan(...)` | `Minimum` (+ `MinimumIsExclusive`) |
 | `LessThanOrEqualTo(...)`, `LessThan(...)` | `Maximum` (+ `MaximumIsExclusive`) |
@@ -110,7 +110,7 @@ The following rules are not mapped, because they don't apply to every instance o
 * Rules under `When(...)` / `Unless(...)` (both the chained and the block form) and their async variants, because the same property can be required for one instance and optional for another.
 * Rules that only belong to a non-default rule set, because ABP validates with FluentValidation's default selector, which does not run them.
 * `RuleForEach(...)` rules, because they constrain the items of a collection rather than the collection property.
-* Comparisons against another property, and any bound that is not a number.
+* Comparisons on a property that is not a number, and comparisons against another property. `Minimum` and `Maximum` are numeric bounds, so the ordinal comparison of two strings can not be published there.
 
 ### Rules That Are Not Fully Expressed
 

framework/src/Volo.Abp.AspNetCore.Mvc/Volo/Abp/AspNetCore/Mvc/AspNetCoreApiDescriptionModelProvider.cs

@@ -341,9 +341,19 @@ public class AspNetCoreApiDescriptionModelProvider : IApiDescriptionModelProvide
             return;
         }
 
+        var propertyInfos = type
+            .GetProperties(BindingFlags.Instance | BindingFlags.Public)
+            .Where(p => p.DeclaringType == type)
+            .ToDictionary(p => p.Name, p => p);
+
         foreach (var propertyModel in typeModel.Properties!)
         {
-            var context = new PropertyApiDescriptionModelContributionContext(propertyModel, type);
+            if (!propertyInfos.TryGetValue(propertyModel.Name, out var propertyInfo))
+            {
+                continue;
+            }
+
+            var context = new PropertyApiDescriptionModelContributionContext(propertyModel, propertyInfo, type);
             foreach (var contributor in _propertyContributors)
             {
                 await contributor.ContributeAsync(context);

framework/src/Volo.Abp.Http.FluentValidation/Volo/Abp/Http/FluentValidation/FluentValidationPropertyApiDescriptionModelContributor.cs

@@ -1,20 +1,38 @@
 using System;
 using System.Collections.Concurrent;
+using System.Collections.Frozen;
 using System.Collections.Generic;
 using System.Globalization;
 using System.Linq;
+using System.Reflection;
 using System.Threading.Tasks;
 using FluentValidation;
 using FluentValidation.Internal;
 using FluentValidation.Validators;
 using Volo.Abp.DependencyInjection;
 using Volo.Abp.Http.Modeling;
+using Volo.Abp.Reflection;
 
 namespace Volo.Abp.Http.FluentValidation;
 
 [ExposeServices(typeof(IPropertyApiDescriptionModelContributor))]
 public class FluentValidationPropertyApiDescriptionModelContributor : IPropertyApiDescriptionModelContributor, ITransientDependency
 {
+    private static readonly FrozenSet<Type> NumericTypes = new HashSet<Type>
+    {
+        typeof(byte),
+        typeof(sbyte),
+        typeof(short),
+        typeof(ushort),
+        typeof(int),
+        typeof(uint),
+        typeof(long),
+        typeof(ulong),
+        typeof(float),
+        typeof(double),
+        typeof(decimal)
+    }.ToFrozenSet();
+
     protected IServiceProvider ServiceProvider { get; }
 
     protected ConcurrentDictionary<Type, ILookup<string, IPropertyValidator>?> RuleCache { get; }
@@ -37,7 +55,7 @@ public class FluentValidationPropertyApiDescriptionModelContributor : IPropertyA
 
         foreach (var validator in rules[context.Model.Name])
         {
-            ApplyValidator(context.Model, validator);
+            ApplyValidator(context.Model, context.PropertyInfo, validator);
         }
 
         return Task.CompletedTask;
@@ -103,7 +121,7 @@ public class FluentValidationPropertyApiDescriptionModelContributor : IPropertyA
             .Any(x => x.IsGenericType && x.GetGenericTypeDefinition() == typeof(ICollectionRule<,>));
     }
 
-    protected virtual void ApplyValidator(PropertyApiDescriptionModel model, IPropertyValidator validator)
+    protected virtual void ApplyValidator(PropertyApiDescriptionModel model, PropertyInfo propertyInfo, IPropertyValidator validator)
     {
         switch (validator)
         {
@@ -118,10 +136,10 @@ public class FluentValidationPropertyApiDescriptionModelContributor : IPropertyA
                 ApplyRegularExpression(model, regularExpressionValidator);
                 break;
             case IBetweenValidator betweenValidator:
-                ApplyBetween(model, betweenValidator);
+                ApplyBetween(model, propertyInfo, betweenValidator);
                 break;
             case IComparisonValidator comparisonValidator:
-                ApplyComparison(model, comparisonValidator);
+                ApplyComparison(model, propertyInfo, comparisonValidator);
                 break;
         }
     }
@@ -156,106 +174,132 @@ public class FluentValidationPropertyApiDescriptionModelContributor : IPropertyA
         model.Regex = validator.Expression;
     }
 
-    protected virtual void ApplyComparison(PropertyApiDescriptionModel model, IComparisonValidator validator)
+    protected virtual void ApplyComparison(PropertyApiDescriptionModel model, PropertyInfo propertyInfo, IComparisonValidator validator)
     {
         switch (validator.Comparison)
         {
             case Comparison.GreaterThan:
-                ApplyMinimum(model, validator.ValueToCompare, isExclusive: true);
+                ApplyMinimum(model, propertyInfo, validator.ValueToCompare, isExclusive: true);
                 break;
             case Comparison.GreaterThanOrEqual:
-                ApplyMinimum(model, validator.ValueToCompare, isExclusive: false);
+                ApplyMinimum(model, propertyInfo, validator.ValueToCompare, isExclusive: false);
                 break;
             case Comparison.LessThan:
-                ApplyMaximum(model, validator.ValueToCompare, isExclusive: true);
+                ApplyMaximum(model, propertyInfo, validator.ValueToCompare, isExclusive: true);
                 break;
             case Comparison.LessThanOrEqual:
-                ApplyMaximum(model, validator.ValueToCompare, isExclusive: false);
+                ApplyMaximum(model, propertyInfo, validator.ValueToCompare, isExclusive: false);
                 break;
         }
     }
 
-    protected virtual void ApplyBetween(PropertyApiDescriptionModel model, IBetweenValidator validator)
+    protected virtual void ApplyBetween(PropertyApiDescriptionModel model, PropertyInfo propertyInfo, IBetweenValidator validator)
     {
         var isExclusive = validator is not IInclusiveBetweenValidator;
 
-        ApplyMinimum(model, validator.From, isExclusive);
-        ApplyMaximum(model, validator.To, isExclusive);
+        ApplyMinimum(model, propertyInfo, validator.From, isExclusive);
+        ApplyMaximum(model, propertyInfo, validator.To, isExclusive);
     }
 
-    protected virtual void ApplyMinimum(PropertyApiDescriptionModel model, object? value, bool isExclusive)
+    protected virtual void ApplyMinimum(PropertyApiDescriptionModel model, PropertyInfo propertyInfo, object? value, bool isExclusive)
     {
-        // Minimum and Maximum are ordered bounds, so a value that is not a number, such as a
-        // DateTime, has nothing meaningful to publish there.
-        if (!TryGetNumber(value, out var minimum))
+        var bound = GetNumericBound(propertyInfo, value);
+        if (bound == null)
         {
             return;
         }
 
         if (model.Minimum != null)
         {
-            if (!TryParseNumber(model.Minimum, out var existingMinimum))
+            if (!TryCompareBounds(model.Minimum, bound, out var comparison))
             {
                 return;
             }
 
-            // The flag follows the winning bound instead of being combined, otherwise ">= 10" merged with "> 5" would become "> 10".
-            var existingIsExclusive = model.MinimumIsExclusive == true;
-            if (existingMinimum > minimum || (existingMinimum == minimum && existingIsExclusive))
+            // The higher bound wins, and an exclusive one is the stricter when both sit on the
+            // same value. The winning bound is published the way it was written, so no value is
+            // lost on the way through a number type that can not hold it.
+            if (comparison > 0 || (comparison == 0 && model.MinimumIsExclusive == true))
             {
-                minimum = existingMinimum;
-                isExclusive = existingIsExclusive;
+                return;
             }
         }
 
-        model.Minimum = minimum.ToString(CultureInfo.InvariantCulture);
+        model.Minimum = bound;
         model.MinimumIsExclusive = isExclusive;
     }
 
-    protected virtual void ApplyMaximum(PropertyApiDescriptionModel model, object? value, bool isExclusive)
+    protected virtual void ApplyMaximum(PropertyApiDescriptionModel model, PropertyInfo propertyInfo, object? value, bool isExclusive)
     {
-        if (!TryGetNumber(value, out var maximum))
+        var bound = GetNumericBound(propertyInfo, value);
+        if (bound == null)
         {
             return;
         }
 
         if (model.Maximum != null)
         {
-            if (!TryParseNumber(model.Maximum, out var existingMaximum))
+            if (!TryCompareBounds(model.Maximum, bound, out var comparison))
             {
                 return;
             }
 
-            var existingIsExclusive = model.MaximumIsExclusive == true;
-            if (existingMaximum < maximum || (existingMaximum == maximum && existingIsExclusive))
+            if (comparison < 0 || (comparison == 0 && model.MaximumIsExclusive == true))
             {
-                maximum = existingMaximum;
-                isExclusive = existingIsExclusive;
+                return;
             }
         }
 
-        model.Maximum = maximum.ToString(CultureInfo.InvariantCulture);
+        model.Maximum = bound;
         model.MaximumIsExclusive = isExclusive;
     }
 
-    protected virtual bool TryGetNumber(object? value, out decimal number)
+    protected virtual string? GetNumericBound(PropertyInfo propertyInfo, object? value)
     {
-        // A comparison against another property has no value to read.
-        var bound = value != null ? Convert.ToString(value, CultureInfo.InvariantCulture) : null;
-        return TryParseNumber(bound, out number);
+        // Minimum and Maximum are numeric bounds. A comparison on another type, the ordinal
+        // comparison of two strings for example, means something else and can not go there.
+        // A comparison against another property has no value to publish either.
+        if (value == null || !NumericTypes.Contains(TypeHelper.StripNullable(propertyInfo.PropertyType)))
+        {
+            return null;
+        }
+
+        var bound = Convert.ToString(value, CultureInfo.InvariantCulture);
+        return bound.IsNullOrWhiteSpace() || !double.TryParse(bound, NumberStyles.Float, CultureInfo.InvariantCulture, out _)
+            ? null
+            : bound;
+    }
+
+    protected virtual bool TryCompareBounds(string left, string right, out int comparison)
+    {
+        // Decimal is exact for every integral type and for decimal itself, which double is not
+        // above its 53 bits of mantissa. Double only comes in for the magnitudes decimal can
+        // not hold, where its precision is the best there is anyway.
+        if (TryParseExactly(left, out var leftValue) && TryParseExactly(right, out var rightValue))
+        {
+            comparison = leftValue.CompareTo(rightValue);
+            return true;
+        }
+
+        if (double.TryParse(left, NumberStyles.Float, CultureInfo.InvariantCulture, out var leftDouble) &&
+            double.TryParse(right, NumberStyles.Float, CultureInfo.InvariantCulture, out var rightDouble))
+        {
+            comparison = leftDouble.CompareTo(rightDouble);
+            return true;
+        }
+
+        comparison = 0;
+        return false;
     }
 
-    protected virtual bool TryParseNumber(string? value, out decimal number)
+    protected virtual bool TryParseExactly(string value, out decimal number)
     {
-        // Float allows the exponent notation but not the group separators, which a Range bound
-        // rendered by a decimal-comma culture would otherwise smuggle in as "1,5" meaning 15.
         if (!decimal.TryParse(value, NumberStyles.Float, CultureInfo.InvariantCulture, out number))
         {
             return false;
         }
 
-        // A magnitude below the decimal range parses to zero, which would publish a bound the
-        // server does not enforce.
-        return number != decimal.Zero || !value!.Any(c => c is > '0' and <= '9');
+        // A magnitude below the decimal range collapses to a zero, which would compare wrong.
+        return number != decimal.Zero || !value.Any(c => c is > '0' and <= '9');
     }
 }

framework/src/Volo.Abp.Http/Volo/Abp/Http/Modeling/PropertyApiDescriptionModelContributionContext.cs

@@ -1,4 +1,5 @@
 using System;
+using System.Reflection;
 using JetBrains.Annotations;
 
 namespace Volo.Abp.Http.Modeling;
@@ -8,14 +9,19 @@ public class PropertyApiDescriptionModelContributionContext
     [NotNull]
     public PropertyApiDescriptionModel Model { get; }
 
+    [NotNull]
+    public PropertyInfo PropertyInfo { get; }
+
     [NotNull]
     public Type DeclaringType { get; }
 
     public PropertyApiDescriptionModelContributionContext(
         [NotNull] PropertyApiDescriptionModel model,
+        [NotNull] PropertyInfo propertyInfo,
         [NotNull] Type declaringType)
     {
         Model = Check.NotNull(model, nameof(model));
+        PropertyInfo = Check.NotNull(propertyInfo, nameof(propertyInfo));
         DeclaringType = Check.NotNull(declaringType, nameof(declaringType));
     }
 }

framework/test/Volo.Abp.Http.FluentValidation.Tests/Volo.Abp.Http.FluentValidation.Tests.csproj

@@ -4,6 +4,7 @@
 
   <PropertyGroup>
     <TargetFramework>net10.0</TargetFramework>
+    <Nullable>enable</Nullable>
     <RootNamespace />
   </PropertyGroup>
 

framework/test/Volo.Abp.Http.FluentValidation.Tests/Volo/Abp/Http/FluentValidation/AbpHttpFluentValidationTestBase.cs

@@ -1,5 +1,6 @@
 using System;
 using System.Linq;
+using System.Reflection;
 using System.Threading.Tasks;
 using Microsoft.Extensions.DependencyInjection;
 using Volo.Abp.Http.Modeling;
@@ -22,9 +23,14 @@ public abstract class AbpHttpFluentValidationTestBase<TStartupModule> : AbpInteg
         var typeModel = TypeApiDescriptionModel.Create(type);
         var contributors = ServiceProvider.GetServices<IPropertyApiDescriptionModelContributor>().ToArray();
 
+        var propertyInfos = type
+            .GetProperties(BindingFlags.Instance | BindingFlags.Public)
+            .Where(p => p.DeclaringType == type)
+            .ToDictionary(p => p.Name, p => p);
+
         foreach (var propertyModel in typeModel.Properties!)
         {
-            var context = new PropertyApiDescriptionModelContributionContext(propertyModel, type);
+            var context = new PropertyApiDescriptionModelContributionContext(propertyModel, propertyInfos[propertyModel.Name], type);
             foreach (var contributor in contributors)
             {
                 await contributor.ContributeAsync(context);

framework/test/Volo.Abp.Http.FluentValidation.Tests/Volo/Abp/Http/FluentValidation/FluentValidationApiDescription_Tests.cs

@@ -76,16 +76,25 @@ public class FluentValidationApiDescription_Tests : AbpHttpFluentValidationTestB
     {
         var property = await GetPropertyAsync<ConstraintTestDto>(nameof(ConstraintTestDto.SmallExponentValue));
 
-        property.Minimum.ShouldBe("0.00000000000000000001");
+        property.Minimum.ShouldBe("1E-20");
     }
 
     [Fact]
-    public async Task Should_Not_Map_A_Bound_Below_The_Decimal_Range()
+    public async Task Should_Map_A_Bound_Below_The_Decimal_Range()
     {
-        // It would parse to a zero, which the server does not accept.
         var property = await GetPropertyAsync<ConstraintTestDto>(nameof(ConstraintTestDto.UnderflowExponentValue));
 
+        property.Minimum.ShouldBe("1E-30");
+    }
+
+    [Fact]
+    public async Task Should_Not_Map_A_Comparison_On_A_Property_That_Is_Not_A_Number()
+    {
+        // The server compares two strings ordinally, so a numeric bound would say something else.
+        var property = await GetPropertyAsync<ConstraintTestDto>(nameof(ConstraintTestDto.StringComparisonValue));
+
         property.Minimum.ShouldBeNull();
+        property.MinimumIsExclusive.ShouldBeNull();
     }
 
     [Fact]
@@ -216,6 +225,17 @@ public class FluentValidationApiDescription_Tests : AbpHttpFluentValidationTestB
         same.MaximumIsExclusive.ShouldBe(true);
     }
 
+    [Fact]
+    public async Task Should_Compare_Bounds_Beyond_The_Double_Precision_Exactly()
+    {
+        // Both bounds collapse to the same double, so only an exact comparison keeps the
+        // stricter attribute bound instead of replacing it with the looser rule.
+        var property = await GetPropertyAsync<DataAnnotationTestDto>(nameof(DataAnnotationTestDto.HighPrecisionValue));
+
+        property.Minimum.ShouldBe("9007199254740993");
+        property.Maximum.ShouldBe("18446744073709551615");
+    }
+
     [Fact]
     public async Task Should_Keep_The_Attribute_Regular_Expression()
     {
@@ -259,6 +279,11 @@ public class FluentValidationApiDescription_Tests : AbpHttpFluentValidationTestB
             var typed = await GetPropertyAsync<CultureTestDto>(nameof(CultureTestDto.TypedDecimalRangeValue));
             typed.Minimum.ShouldBe("2");
             typed.Maximum.ShouldBe("9.5");
+
+            // A bound outside the decimal range still loses to the stricter rule.
+            var exponent = await GetPropertyAsync<CultureTestDto>(nameof(CultureTestDto.ExponentRangeValue));
+            exponent.Minimum.ShouldBe("2");
+            exponent.Maximum.ShouldBe("1E+30");
         }
     }
 

framework/test/Volo.Abp.Http.FluentValidation.Tests/Volo/Abp/Http/FluentValidation/TestObjects/ConstraintTestDto.cs

@@ -46,6 +46,8 @@ public class ConstraintTestDto
 
     public double UnderflowExponentValue { get; set; }
 
+    public string? StringComparisonValue { get; set; }
+
     public string? UnconstrainedValue { get; set; }
 }
 
@@ -73,5 +75,6 @@ public class ConstraintTestDtoValidator : AbstractValidator<ConstraintTestDto>
         RuleFor(x => x.DynamicLengthValue).Length(_ => 2, _ => 8);
         RuleFor(x => x.SmallExponentValue).GreaterThanOrEqualTo(1e-20);
         RuleFor(x => x.UnderflowExponentValue).GreaterThanOrEqualTo(1e-30);
+        RuleFor(x => x.StringComparisonValue).GreaterThan("10");
     }
 }

framework/test/Volo.Abp.Http.FluentValidation.Tests/Volo/Abp/Http/FluentValidation/TestObjects/CultureTestDto.cs

@@ -12,6 +12,9 @@ public class CultureTestDto
     [Range(typeof(decimal), "1,5", "9,5")]
     public decimal TypedDecimalRangeValue { get; set; }
 
+    [Range(typeof(double), "1e-30", "1e30", ParseLimitsInInvariantCulture = true)]
+    public double ExponentRangeValue { get; set; }
+
     [Range(typeof(DateTime), "2020-01-01", "2030-01-01")]
     public DateTime DateRangeValue { get; set; }
 }
@@ -22,5 +25,6 @@ public class CultureTestDtoValidator : AbstractValidator<CultureTestDto>
     {
         RuleFor(x => x.DecimalRangeValue).GreaterThanOrEqualTo(2.0);
         RuleFor(x => x.TypedDecimalRangeValue).GreaterThanOrEqualTo(2m);
+        RuleFor(x => x.ExponentRangeValue).GreaterThanOrEqualTo(2d);
     }
 }

framework/test/Volo.Abp.Http.FluentValidation.Tests/Volo/Abp/Http/FluentValidation/TestObjects/DataAnnotationTestDto.cs

@@ -23,6 +23,9 @@ public class DataAnnotationTestDto
 
     [Range(1, 100, MinimumIsExclusive = true, MaximumIsExclusive = true)]
     public int ExclusiveAttributeValue { get; set; }
+
+    [Range(typeof(ulong), "9007199254740993", "18446744073709551615", ParseLimitsInInvariantCulture = true)]
+    public ulong HighPrecisionValue { get; set; }
 }
 
 public class DataAnnotationTestDtoValidator : AbstractValidator<DataAnnotationTestDto>
@@ -34,5 +37,6 @@ public class DataAnnotationTestDtoValidator : AbstractValidator<DataAnnotationTe
         RuleFor(x => x.MergedRangeValue).GreaterThanOrEqualTo(10).LessThanOrEqualTo(90);
         RuleFor(x => x.LooserFluentBoundValue).GreaterThan(-5).LessThan(500);
         RuleFor(x => x.SameBoundValue).GreaterThan(10).LessThan(90);
+        RuleFor(x => x.HighPrecisionValue).GreaterThanOrEqualTo(9_007_199_254_740_992UL);
     }
 }