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
- 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; }
}
- 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();
- 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;
}
}
- 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)
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 #90275Just 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:
Install-Package DataAnnotationsValidator
or copying the code from the repositoryUsage should be simple - just replace calls of
ValidateDataAnnotations
withValidateDataAnnotationsRecursively
.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 propertiesValidateOnStart
: call Microsoft.Extensions.Hosting method in version 6, but custom code in previous .NET versionAnd a
ServiceCollection
extension method:which is syntactic sugar for
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 inSystem.ComponentModel.DataAnnotations
or inMicrosoft.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.)
Use as follows:
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?)
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.