Safer recursive validation.

[niemyjski]

If you have dynamic payloads like JsonPatch / Delta<T> or content that may be a JToken or other Json Type MiniValidator throws quickly. Can we add some type guards or safety to continue on when an error occurs?

"message": "Cannot access child value on Newtonsoft.Json.Linq.JValue.",
"is_error": true,
"detail": "   at Newtonsoft.Json.Linq.JToken.get_First()\r\n   at MiniValidation.PropertyHelper.CallNullSafePropertyGetter[TDeclaringType,TValue](Func`2 getter, Object target) in /_/src/MiniValidation/PropertyHelper.cs:line 129\r\n   at MiniValidation.PropertyDetails.GetValue(Object target) in /_/src/MiniValidation/TypeDetailsCache.cs:line 268\r\n   at MiniValidation.MiniValidator.TryValidateImpl(Object target, IServiceProvider serviceProvider, Boolean recurse, Boolean allowAsync, Dictionary`2 workingErrors, Dictionary`2 validatedObjects, List`1 validationResults, String prefix, Int32 currentDepth) in /_/src/MiniValidation/MiniValidator.cs:line 383\r\n   at MiniValidation.MiniValidator.TryValidateEnumerable(Object target, IServiceProvider serviceProvider, Boolean recurse, Boolean allowAsync, Dictionary`2 workingErrors, Dictionary`2 validatedObjects, List`1 validationResults, String pref

[DamianEdwards]

@niemyjski any reason you can't skip recursion for the property using [SkipRecursion]? I can't take a dependency on JSON.NET and seems odd to exclude it specifically based on string match of type name. Would be good to know if the same scenario throws in MVC/Blazor as they both have custom recursive validation too, and if it doesn't throw, why?

[lukeskrz]

I think I just ran into something similar. Here is a simple repro:

using System.Text.Json;

public class TestOptions
{
    public JsonSerializerOptions JsonSerializerOptions { get; set; } = new();
}

When calling MiniValidator.TryValidate(options, out var validationErrors), it throws an InvalidOperationException:

System.InvalidOperationException
  HResult=0x80131509
  Message=Method may only be called on a Type for which Type.IsGenericParameter is true.
  Source=System.Private.CoreLib
  StackTrace:
   at System.RuntimeType.get_DeclaringMethod() in /_/src/coreclr/System.Private.CoreLib/src/System/RuntimeType.CoreCLR.cs:line 3262
   at MiniValidation.PropertyHelper.CallNullSafePropertyGetter[TDeclaringType,TValue](Func`2 getter, Object target) in /_/src/MiniValidation/PropertyHelper.cs:line 129
   at MiniValidation.PropertyDetails.GetValue(Object target) in /_/src/MiniValidation/TypeDetailsCache.cs:line 268
   at MiniValidation.MiniValidator.<TryValidateImpl>d__20.MoveNext() in /_/src/MiniValidation/MiniValidator.cs:line 383
   at System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable`1.ConfiguredValueTaskAwaiter.GetResult() in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConfiguredValueTaskAwaitable.cs:line 154
   at MiniValidation.MiniValidator.<TryValidateImpl>d__20.MoveNext() in /_/src/MiniValidation/MiniValidator.cs:line 446
   at System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable`1.ConfiguredValueTaskAwaiter.GetResult() in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConfiguredValueTaskAwaitable.cs:line 154
   at MiniValidation.MiniValidator.<TryValidateEnumerable>d__25.MoveNext() in /_/src/MiniValidation/MiniValidator.cs:line 572
   at System.Runtime.CompilerServices.ValueTaskAwaiter`1.GetResult() in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ValueTaskAwaiter.cs:line 126
   at MiniValidation.MiniValidator.<TryValidateEnumerable>d__24.MoveNext() in /_/src/MiniValidation/MiniValidator.cs:line 538
   at System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable`1.ConfiguredValueTaskAwaiter.GetResult() in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConfiguredValueTaskAwaitable.cs:line 154
   at MiniValidation.MiniValidator.<TryValidateImpl>d__20.MoveNext() in /_/src/MiniValidation/MiniValidator.cs:line 439
   at System.Runtime.CompilerServices.ConfiguredValueTaskAwaitable`1.ConfiguredValueTaskAwaiter.GetResult() in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ConfiguredValueTaskAwaitable.cs:line 154
   at MiniValidation.MiniValidator.<TryValidateImpl>d__20.MoveNext() in /_/src/MiniValidation/MiniValidator.cs:line 446
   at System.Runtime.CompilerServices.ValueTaskAwaiter`1.GetResult() in /_/src/libraries/System.Private.CoreLib/src/System/Runtime/CompilerServices/ValueTaskAwaiter.cs:line 126
   at MiniValidation.MiniValidator.TryValidateImpl[TTarget](TTarget target, IServiceProvider serviceProvider, Boolean recurse, Boolean allowAsync, IDictionary`2& errors) in /_/src/MiniValidation/MiniValidator.cs:line 200
   at MiniValidation.MiniValidator.TryValidate[TTarget](TTarget target, IDictionary`2& errors) in /_/src/MiniValidation/MiniValidator.cs:line 59

Note that this worked fine on Net 6, but started breaking after upgrading to Net 8.

For us, the problem with using [SkipRecursion] is that the options class is in a separate shared nuget package, and we'd prefer not to add a dependency in that package on MiniValidation.

[DamianEdwards]

I could add support for System.Runtime.Serialization.IgnoreDataMemberAttribute for skipping members too, so you wouldn't need to take a dependency on MiniValidation to annotate members to skip.

Can either of you determine if MVC has issues when accepting these types as input that gets validated? I'm curious what its behavior is.

[lukeskrz]

Using ValidateDataAnnotations seems to have no problem with the same options class:

services.AddOptions<TestOptions>()
            .BindConfiguration("TestOptions")
            .ValidateDataAnnotations()
            .ValidateOnStart();

[DamianEdwards]

I don't think ValidateDataAnnotations does recursive validation though. The behavior we're seeing in MiniValidation is because it traverses complex object graphs by default and supports poly-morphism of runtime types. That's why it tries to recurse into these types that throw on their getters. MVC validation is similar so if it behaves differently we can dig deeper to understand exactly what it's doing differently. If it behaves the same way then I'll have to consider introducing some behavior (maybe behind a flag) that prevents the exceptions during recursion.

[lukeskrz]

Assuming I'm not missing something, the model validation seems to work normally:

using System.Text.Json;

public class TestOptions
{
    public JsonSerializerOptions JsonSerializerOptions { get; set; } = new();
}

...

[HttpGet]
[Route("test")]
public IActionResult Test(TestOptions options)
{
    if (!ModelState.IsValid) // this returns true
    {
        throw new InvalidOperationException();
     }
}

I also tested again with DataAnnotations and the new ValidateObjectMembers attribute (which enables recursive validation on a member; https://github.com/dotnet/runtime/pull/90275), and that also seems to work.

[DamianEdwards]

OK thanks. The new ValidateObjectMembers and ValidateEnumeratedItems attributes only work for options validation I believe so their existence on a model member has no effect in MVC or MiniValidator.

I'll have to dig in to what MVC is doing that results in the getter of the JsonSerialerOptions either not being accessed or being accessed and the exception being dealt with.