Skip to content

Add support for validating complex or collection properties using System.ComponentModel.DataAnnotations.Validator #31400

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
pranavkm opened this issue Nov 5, 2019 · 3 comments
Assignees
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.ComponentModel.DataAnnotations
Milestone

Comments

@pranavkm
Copy link
Contributor

pranavkm commented Nov 5, 2019

Background

Validator.TryValidateObject does not support validating complex sub-properties or collection types. From the docs:

It validates the property values of the object if validateAllProperties is true but does not recursively validate properties of the objects returned by the properties.

Validation attributes or IValidatableObject contracts applied to sub-properties or entries in collection properties are ignored by this API. Users have been working around this by developing their own solutions such as the one described here - http://www.technofattie.com/2011/10/05/recursive-validation-using-dataannotations.html. MVC supports DataAnnotations attributes applied to models, but does it's own graph walking so doesn't have to deal with this issue.

Blazor on the other hand relies on the Validator.TryValidateObject API for it's validation. We've had reports where users are confused \ surprised by this behavior. For the 3.1 \ 3.2 releases, we're shipping an experimental package - https://www.nuget.org/packages/Microsoft.AspNetCore.Components.DataAnnotations.Validation/ with APIs that support validating complex \ collection type properties. For the 5.0 release, we'd like for the runtime to support this feature.

API proposal

Basing this proposal based on the experimental package that's shipping as part of Blazor

Introduce a new attribute ValidateComplexTypeAttribute that allows Validator to recurse in to complex or collection type properties. It's important to note that this only recurses one level. This avoids coming up strategies to limit traversing in to arbitrary object graphs.

We introduce overloads to Validator.TryValidateObject and Validator.ValidateObject that allows navigating in to properties that are ValidateComplexTypeAttribute. The documentation for the existing overloads explicitly state that they does not recurse in to properties. Changing it's behavior isn't desirable given how stable this API has been.

Usage

class Person
{
     ....
    [ValidateComplexType]
    public Address Address { get; set; } = new Address();
}

Person person = GetPerson();
Validator.TryValidateObject(person);

API diff

namespace System.ComponentModel.DataAnnotations
{
   public class ValidateComplexTypeAttribute : ValidationAttribute {}

    public static class Validator
    {
      public static bool TryValidateObject(object instance, System.ComponentModel.DataAnnotations.ValidationContext validationContext, System.Collections.Generic.ICollection<System.ComponentModel.DataAnnotations.ValidationResult> validationResults, bool validateAllProperties, bool validateComplexTypeProperties) { throw null; }

      public static void ValidateObject(object instance, System.ComponentModel.DataAnnotations.ValidationContext validationContext, bool validateAllProperties, bool validateComplexTypeProperties) { }
    }
}
@msftgits msftgits transferred this issue from dotnet/corefx Feb 1, 2020
@maryamariyan maryamariyan added the untriaged New issue has not been triaged by the area owner label Feb 23, 2020
@terrajobst terrajobst added api-ready-for-review and removed untriaged New issue has not been triaged by the area owner labels May 15, 2020
@terrajobst
Copy link
Contributor

Tagging area owners @lajones @ajcvickers

@terrajobst terrajobst added api-suggestion Early API idea and discussion, it is NOT ready for implementation and removed api-ready-for-review labels May 15, 2020
@pranavkm pranavkm added api-ready-for-review blocking Marks issues that we want to fast track in order to unblock other important work and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jun 9, 2020
@bartonjs
Copy link
Member

bartonjs commented Jun 18, 2020

Video

  • The new attribute seems fine, but be careful in how it gets consumed so as to not surprise users who use the objects in multiple contexts (Blazor, EF6, etc).
  • It seems like some new data belongs on ValidationContext, instead of new overloads to (Try)ValidateObject, around memberName-path representations and other handling of [ValidateComplexType].

@bartonjs bartonjs added api-suggestion Early API idea and discussion, it is NOT ready for implementation api-needs-work API needs work before it is approved, it is NOT ready for implementation and removed api-ready-for-review api-suggestion Early API idea and discussion, it is NOT ready for implementation blocking Marks issues that we want to fast track in order to unblock other important work labels Jun 18, 2020
@ajcvickers ajcvickers added this to the 5.0.0 milestone Jun 22, 2020
@ajcvickers ajcvickers modified the milestones: 5.0.0, Future Jul 9, 2020
@pranavkm
Copy link
Contributor Author

Closing this as we're going to work on addressing this within Blazor.

@ghost ghost locked as resolved and limited conversation to collaborators Apr 29, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
api-needs-work API needs work before it is approved, it is NOT ready for implementation area-System.ComponentModel.DataAnnotations
Projects
None yet
Development

No branches or pull requests

5 participants