runtime: [Extensions.Options] ValidateDataAnnotation doesn't work when classes have properties that also have DataAnnotations on their members

Describe the bug

When using ValidateDataAnnotations to validate a configuration hierarchy, you can only validate top level members of the class. Any properties that are types which also include validation are NOT validated.

Perhaps this was viewed as a design decision, but I think it’d be a lot more consistent if automatic data annotation validation worked in the same fashion as mvc model validation.

To Reproduce

  1. Design the classes which represent the nested configuration
public class AnnotatedOptions
{
    [Required]
    public string Required { get; set; }

    [StringLength(5, ErrorMessage = "Too long.")]
    public string StringLength { get; set; }

    [Range(-5, 5, ErrorMessage = "Out of range.")]
    public int IntRange { get; set; }

    public AnnotatedOptionsSubsection AnnotatedOptionSubsection { get; set; }
}

public class AnnotatedOptionsSubsection
{
    [Range(-5, 5, ErrorMessage = "Really out of range.")]
    public int IntRange2 { get; set; }
}
  1. Register the options
var services= new ServiceCollection();
services
     .AddOptions<AnnotatedOptions>()
     .Configure(o =>
                {
                    o.StringLength = "111111";
                    o.IntRange = 10;
                    o.AnnotatedOptionSubsection = new AnnotatedOptionsSubsection
                    {
                        IntRange2 = 10000
                    };
                })
       .ValidateDataAnnotations();
  1. Set up a controller which retrieves the options
[Route("api/[controller]")]
[ApiController]
public class ConfigController : ControllerBase
{
    private readonly AnnotatedOptions _configuration;

    public ConfigController(IOptionsMonitor<AnnotatedOptions> configuration)
    {
        try
        {
            _configuration = configuration.CurrentValue;
        }
        catch (OptionsValidationException ex)
        {
            throw new Exception(string.Join(@"\r\n", ex.Failures), ex);
        }
    }

    // GET api/config/
    [HttpGet("{configKey}")]
    public ActionResult<AnnotatedOptions> Get()
    {
        return _configuration;
    }
}
  1. Invoke the controller method and observe the error results
System.Exception: DataAnnotation validation failed for members Required with the error 'The Required field is required.'.
DataAnnotation validation failed for members StringLength with the error 'Too long.'.
DataAnnotation validation failed for members IntRange with the error 'Out of range.'. ---> Microsoft.Extensions.Options.OptionsValidationException: Exception of type 'Microsoft.Extensions.Options.OptionsValidationException' was thrown.
   at Microsoft.Extensions.Options.OptionsFactory`1.Create(String name)
   at Microsoft.Extensions.Options.OptionsMonitor`1.<>c__DisplayClass10_0.<Get>b__0()
   at System.Lazy`1.ViaFactory(LazyThreadSafetyMode mode)
   at System.Lazy`1.ExecutionAndPublication(LazyHelper executionAndPublication, Boolean useDefaultConstructor)
   at System.Lazy`1.CreateValue()
   at Microsoft.Extensions.Options.OptionsCache`1.GetOrAdd(String name, Func`1 createOptions)
   at Microsoft.Extensions.Options.OptionsMonitor`1.Get(String name)
   at Microsoft.Extensions.Options.OptionsMonitor`1.get_CurrentValue()
   at PF.ClinicalEndpoint.Host.Controllers.ConfigController..ctor(IOptionsMonitor`1 configuration) in C:\dev\test\ClinicalEndpoint\src\Host\Controllers\ConfigController.cs:line 19
   --- End of inner exception stack trace ---
   at PF.ClinicalEndpoint.Host.Controllers.ConfigController..ctor(IOptionsMonitor`1 configuration) in C:\dev\test\ClinicalEndpoint\src\Host\Controllers\ConfigController.cs:line 23
   at lambda_method(Closure , IServiceProvider , Object[] )
   at Microsoft.AspNetCore.Mvc.Controllers.ControllerActivatorProvider.<>c__DisplayClass4_0.<CreateActivator>b__0(ControllerContext controllerContext)
   at Microsoft.AspNetCore.Mvc.Controllers.ControllerFactoryProvider.<>c__DisplayClass5_0.<CreateControllerFactory>g__CreateController|0(ControllerContext controllerContext)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ControllerActionInvoker.InvokeInnerFilterAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeNextResourceFilter()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Rethrow(ResourceExecutedContext context)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeFilterPipelineAsync()
   at Microsoft.AspNetCore.Mvc.Internal.ResourceInvoker.InvokeAsync()
   at Microsoft.AspNetCore.Routing.EndpointMiddleware.Invoke(HttpContext httpContext)
   at Microsoft.AspNetCore.Routing.EndpointRoutingMiddleware.Invoke(HttpContext httpContext)
   at PF.Core.WebApi.Middleware.ExceptionHandlingMiddleware.InvokeAsync(HttpContext httpContext)
   at PF.Core.WebApi.Middleware.RequestLoggingMiddleware.InvokeAsync(HttpContext httpContext)
   at Microsoft.AspNetCore.Diagnostics.DeveloperExceptionPageMiddleware.Invoke(HttpContext context)
```

### Expected behavior
Another error in the list of failures for the `IntRange2` field which reads as follows
`DataAnnotation validation failed for members IntRange2 with the error 'Really out of range.'`

About this issue

  • Original URL
  • State: closed
  • Created 5 years ago
  • Reactions: 39
  • Comments: 28 (14 by maintainers)

Most upvoted comments

I know that it’s possible to fix, but this should be part of the .NET runtime. Not an extension method I need to copy to every application I ever work on.

It seems like this will be fixed in .NET 8 with the new attribute ValidateObjectMembersAttribute, see #90275

Just came across this issue and am quite flabberghasted and annoyed that recursive validation does not take place nor is there an option to set it. It has cost me a decent chunk of time trying to figure out why my tests were failing.

I echo the request to at least document this to save others frustration time.

Another question would be why this was removed from the 6.0.0 milestone. Are inclusions in these milestones not commitments but “best effort” ?

@maryamariyan It feels very weird to me to include https://github.com/dotnet/runtime/issues/36391 in release 6 but not this issue. I guess it’s too late now though. An idea is to at least add to the documentation for Options Validation that it (currently) only validates fields on the top Options class, not fields in nested Options classes.

It would be good to address this in 7.0.

Found a workaround for nested objects based on similar StackOveflow question and corresponding Nuget package. It works for nested objects, arrays and dictionaries but requires some preparation:

  • Installing the package: Install-Package DataAnnotationsValidator or copying the code from the repository
  • Creating custom options validator:
public class RecursiveDataAnnotationValidateOptions<TOptions>: IValidateOptions<TOptions> where TOptions: class
{
    private readonly DataAnnotationsValidator.DataAnnotationsValidator _validator = new();

    public RecursiveDataAnnotationValidateOptions(string name) => Name = name;

    public string Name { get; }

    public ValidateOptionsResult Validate(string name, TOptions options)
    {
        if (name != Name)
            return ValidateOptionsResult.Skip;

        var results = new List<ValidationResult>();
        if (_validator.TryValidateObjectRecursive(options, results))
            return ValidateOptionsResult.Success;

        var stringResult = results.Select(validationResult =>
            $"DataAnnotation validation failed for members: '{string.Join((string?)",", (IEnumerable<string?>)validationResult.MemberNames)}' with the error: '{validationResult.ErrorMessage}'."
        ).ToList();

        return ValidateOptionsResult.Fail(stringResult);
    }
}
  • Creating extension method for it:
public static OptionsBuilder<T> ValidateDataAnnotationsRecursively<T>(this OptionsBuilder<T> builder) where T: class
{
    builder.Services.AddSingleton<IValidateOptions<T>>(new RecursiveDataAnnotationValidateOptions<T>(builder.Name));
    return builder;
}

Usage should be simple - just replace calls of ValidateDataAnnotations with ValidateDataAnnotationsRecursively.

Hey, any news with solution for that issue?

For Options validation, you can use this NuGet package (source and docs (GitHub)) until the release of .NET 7 (or 8, 9…).

It is compatible with .NET Standard 2.0, .NET Core 3.1, .NET 5 and .NET 6.

It provides two OptionBuilder extension methods:

  • ValidateDataAnnotationsRecursively: based on Microsoft.Extensions.Options.DataAnnotations, but it can even validate nested properties
  • ValidateOnStart: call Microsoft.Extensions.Hosting method in version 6, but custom code in previous .NET version

And a ServiceCollection extension method:

services.ConfigureAndValidate<TOptions>(configureOptions)

which is syntactic sugar for

services
    .AddOptions<TOptions>()
        .Configure(configureOptions) // Microsoft
        .ValidateDataAnnotationsRecursively() // based on Microsoft's ValidateDataAnnotations, but supports nested properties
        .ValidateOnStart() // Microsoft (.NET 6) or ValidateEagerly() in previous versions
        .Services

https://github.com/dotnet/runtime/issues/85475 is adding a ValidateObjectMembersAttribute which would handle this case. There is a question about whether that attribute should live in System.ComponentModel.DataAnnotations or in Microsoft.Extensions.Options.

cc @jeffhandley @DamianEdwards @tarekgh

I’m still interested in having this issue addressed.

I also came across this issue and would like to have seen at least a large disclaimer that this does not work as one would reasonably expect from a released (finished?) product. I agree with https://github.com/dotnet/runtime/issues/36093#issuecomment-755586930

I came up with my own workaround, where the only requirement is that all option types are defined in the same namespace. It’s tuned to my specific needs, but maybe it is useful to someone. 🙂

(The namespace requirement is just one way to prioritize specificity over sensitivity - matching e.g. a string is evidently wrong and will even result in a runtime exception. Finer tuned approaches could check for the presence of annotations on the members, for example.)

public static class ServiceConfiguration
{
  private static readonly MethodInfo HelperMethod =
    typeof(ServiceConfiguration).GetMethod(nameof(AddValidatedOptions), BindingFlags.Static | BindingFlags.NonPublic)
    ?? throw new InvalidOperationException("famously, this should not happen");

  public static IServiceCollection AddValidatedOptions<TOptions>(this IServiceCollection services, string sectionPath)
    where TOptions : class
  {
    var @namespace = typeof(TOptions).Namespace
                     ?? throw new ArgumentException("could not get namespace of type", nameof(TOptions));
    AddValidatedOptions<TOptions>(services, sectionPath, @namespace);
    return services;
  }

  private static void AddValidatedOptions<TOptions>(IServiceCollection services, string sectionPath, string @namespace)
    where TOptions : class
  {
    services
      .AddOptions<TOptions>()
      .BindConfiguration(sectionPath)
      .ValidateDataAnnotations()
      .ValidateOnStart();

    foreach (var pi in typeof(TOptions).GetProperties(BindingFlags.Instance | BindingFlags.Public))
    {
      bool skip = !pi.PropertyType.IsClass
                  || string.CompareOrdinal(pi.PropertyType.Namespace, @namespace) != 0;
      if (skip)
      {
        continue;
      }

      string currentPath = string.Join(':', sectionPath, pi.Name);
      HelperMethod.MakeGenericMethod(pi.PropertyType)
        .Invoke(null, new object[] { services, currentPath, @namespace });
    }
  }
}

Use as follows:

services.AddValidatedOptions<ComplexOption>("path:to:complex:option:that:is:probably:way:shorter:than:this:example");

Lastly, this is a recursive implementation, so don’t go and use this with configuration files that nest … a lot of objects. (What kind of monster would do that?)

I know that it’s possible to fix, but this should be part of the .NET runtime. Not an extension method I need to copy to every application I ever work on.

Options is part of every applications. Some may have lots of. Nesting options is obvious in this case as it helps sorting them. It is even the purpose of the Option pattern. And adding lots of options in the DI to avoid nesting them is not a good idea because you may have a service using many of them and this will result in a method declaration with lots of parameters for each options you need instead having only one option “registry” you can navigate into.