Recursive validation for Options

[SteveDunn]

Fixes #36093

[ghost]

Tagging subscribers to this area: @dotnet/area-extensions-options
See info in area-owners.md if you want to be subscribed.

Issue Details

---

Fixes #36093

Author:	SteveDunn
Assignees:	-
Labels:	area-Extensions-Options
Milestone:	-

[eerhardt]

I think this should be solved at a lower level. I don't think this is the correct level to solve this. If we have scenarios for Validator to validate complex types, we should fix it in System.ComponentModel.DataAnnotations.

See related:

• Add support for validating complex or collection properties using System.ComponentModel.DataAnnotations.Validator #31400
• Consider shipping ObjectGraphDataAnnotationsValidator / ValidateComplexTypeAttribute aspnetcore#28640

[SteveDunn]

I think this should be solved at a lower level. I don't think this is the correct level to solve this. If we have scenarios for Validator to validate complex types, we should fix it in System.ComponentModel.DataAnnotations.

See related:

• Add support for validating complex or collection properties using System.ComponentModel.DataAnnotations.Validator #31400
• Consider shipping ObjectGraphDataAnnotationsValidator / ValidateComplexTypeAttribute aspnetcore#28640

From reading the original Issue, I was under the impression that this should be limited to Options and not validation in general. I'm happy to look at implementing it in that layer. Is there a consensus on this approach?

[eerhardt]

Is there a consensus on this approach?

@ajcvickers @lajones - thoughts here? This seems like a commonly requested feature in DataAnnotations. ASP.NET Blazor wants it, multiple members of the community want it here and here, and here Options validation wants it.

My thinking is this should be solved in the lower level so upstack callers don't need to create multiple different solutions to this problem.

cc @jeffhandley

[eerhardt]

Pinging @SteveSandersonMS as well - since he may have some insight on how ASP.NET plans on supporting validating complex properties.

[SteveSandersonMS]

While we're adding people, I'll bring in @DamianEdwards who's been working on a validator library recently and will likely have opinions about what our product goals should be in this area.

Our current non-official solution (ObjectGraphDataAnnotationsValidator) remains a candidate for .NET 7 planning - dotnet/aspnetcore#28640.

[DamianEdwards]

Thanks @SteveSandersonMS. The library is MiniValidation and indeed it supports validating complex properties. It's built atop the System.ComponentModel.DataAnnotations.Validator class. The other thing it aims to provide is a single line calling pattern (MiniValidator.TryValidate(object target, out IDictionary<string, string[]> errors) and providing the results in a JSON Problem Details friendly shape (IDictionary<string, string[]>).

I agree with @eerhardt that ideally we should solve this in the System.ComponentModel.DataAnnotations layer so that all frameworks that leverage the types there for validation could realize the benefit. The calling pattern there is still somewhat cumbersome and inefficient but individual frameworks tend to hide the former anyway and for cases like ASP.NET Core Minimal APIs where the methods would need to be called directly, libraries like mine could act as the nicer interaction model.

[SteveDunn]

I'm happy to move this to System.ComponentModel.DataAnnotations then. Would this work still fit within the remit of the original Issue, or would a new Issue, and a potential API Proposal need creating?

[eerhardt]

Would this work still fit within the remit of the original Issue, or would a new Issue, and a potential API Proposal need creating?

@ajcvickers - thoughts?

I think the original issue proposed by @pranavkm had some advantages. Namely, it doesn't recurse blindly - it only follows complex properties if the model author opts-in to the complex property being validated as well.

[DamianEdwards]

@eerhardt if we wish to have these changes be consumable by frameworks like ASP.NET Core MVC, Blazor, etc. then requiring the attribute might make that difficult as existing models won't have it. Perhaps we could consider APIs for both patterns, i.e. default mode which only recurses if attribute is found, opt-in mode that enables recursion at the call site (or forcibly disables it even ignoring the attribute if found).

It would be nice to consider a simpler calling pattern too while we're looking at this part of the framework.

[eerhardt]

My thinking was that it is the model author that decides what should and shouldn't be validated. It isn't really up to ASP.NET MVC or Blazor, is it? The frameworks just say "validate this object", and the object's attributes are what declares what should and shouldn't be validated.

as existing models won't have it

That was my intention. The behavior is the same until the model is updated to say "drill into this property and validate it as well".

[DamianEdwards]

@eerhardt my point was these frameworks already do recursive validation of complex properties today, without the attribute, and that if we update System.ComponentModel.DataAnnotations to support recursive validation it would be good to be able to use it while maintaining the existing framework default behavior.

[maryamariyan]

Closing this PR as per conversation above. Will take this on in the issue page and relabed to System.ComponentModel.DataAnnotations

[SteveDunn]

@maryamariyan @eerhardt @SteveSandersonMS @DamianEdwards - new PR with this moved down to System.Component.DataAnnotations: #67810