Skip to content
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

Consider shipping ObjectGraphDataAnnotationsValidator / ValidateComplexTypeAttribute #28640

Open
pranavkm opened this issue Dec 14, 2020 · 47 comments
Labels
affected-medium This issue impacts approximately half of our customers area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one Pillar: Complete Blazor Web Priority:2 Work that is important, but not critical for the release severity-major This label is used by an internal tool
Milestone

Comments

@pranavkm
Copy link
Contributor

It's not entirely clear how we would solve dotnet/runtime#31400. One alternative is to ship the experimental validation features as part of Blazor instead.

@pranavkm pranavkm added this to the Next sprint planning milestone Dec 14, 2020
@ghost
Copy link

ghost commented Dec 14, 2020

Thanks for contacting us.
We're moving this issue to the Next sprint planning milestone for future evaluation / consideration. We will evaluate the request when we are planning the work for the next milestone. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@pranavkm pranavkm added the area-blazor Includes: Blazor, Razor Components label Dec 14, 2020
@mkArtakMSFT mkArtakMSFT added the enhancement This issue represents an ask for new feature or an enhancement to an existing one label Jan 25, 2021
@SteveSandersonMS SteveSandersonMS added affected-medium This issue impacts approximately half of our customers severity-major This label is used by an internal tool labels Jan 26, 2021 — with ASP.NET Core Issue Ranking
@ghost
Copy link

ghost commented Jul 20, 2021

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

@javiercn javiercn added the jcn-p0 label Nov 5, 2021
@mkArtakMSFT mkArtakMSFT added the Priority:1 Work that is critical for the release, but we could probably ship without label Nov 23, 2021
@mkArtakMSFT mkArtakMSFT modified the milestones: Backlog, .NET 7 Planning Nov 23, 2021
@ghost
Copy link

ghost commented Nov 23, 2021

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@Duranom
Copy link

Duranom commented Jan 31, 2022

Could the ValidationComplexTypeAttribute also be considered to not only work through ObjectGraphDataAnnotationValidator when looking at this again? Maybe even bringing it into the System.ComponentModel.DataAnnotations itself?
Asking this as complex type validation is not uncommon with WPF, Forms and other places where data annotation is used into account and having to deal several solutions that try do this.
There are other frameworks for validation and packages that try to do the same, but one would expect something more from out of the box, especially when moving and modernizing applications that use DataAnnotations to the latest NET versions / Blazor / Maui this seems to popping up as an point of discussion sometimes.

@boukenka
Copy link

Could you tell us if this is planned for a specific preview? Or if it will be soon?

@boukenka
Copy link

@pranavkm Hello. Could you tell us if this is planned for a specific preview? Or if it will be soon?

@boukenka
Copy link

boukenka commented Jun 7, 2022

@danroth27 Hello. Could you tell us if this is planned for a specific preview? Or if it will be soon? What is the current status?

@boukenka
Copy link

boukenka commented Jul 14, 2022

@TanayParikh Hello. I am trying to get an answer. Do you know if this issue is planned for a specific preview? Or if it will be soon? What is the current status? Is it moved to .NET 8? I know that for now, it is included in the planning of .NET 7.

@TanayParikh
Copy link
Contributor

I know that for now, it is included in the planning of .NET 7.

That's all the information I have. We only have the RCs remaining before .NET 7 GAs. Unclear if this'll make it in the RCs or not. We'll update this issue when that decision is made.

@boukenka
Copy link

boukenka commented Jul 14, 2022

I know that for now, it is included in the planning of .NET 7.

That's all the information I have. We only have the RCs remaining before .NET 7 GAs. Unclear if this'll make it in the RCs or not. We'll update this issue when that decision is made.

Thank you for your quick answer @TanayParikh

@billreiss
Copy link

I'll like to add a +1 that I'd really like to see this get released.

@SkynetSudo
Copy link

SkynetSudo commented Nov 7, 2023

No update on this issue, can this issue be included in .NET9 Planning and resolved with the release.

@diptim01
Copy link

Well, one has to look for alternatives at this point. Rolling out a custom validator is extra work

@ite-klass
Copy link

ite-klass commented Nov 20, 2023

@diptim01 Where do you see the extra work? More than assessing and including alternative dependencies?

Copying the source of the last released version is a good starting point and works well out of the box. It's only two classes you need.

The references and my xdoc for your convenience:

/// <summary>
/// A <see cref="ValidationAttribute"/> that indicates that the property is a complex or collection type that further needs to be validated.
/// <para>
/// By default <see cref="Validator"/> does not recurse in to complex property types during validation.
/// When used in conjunction with <see cref="ObjectGraphDataAnnotationsValidator"/>, this property allows the validation system to validate
/// complex or collection type properties.
/// </para>
/// </summary>
/// <seealso cref="https://github.com/dotnet/aspnetcore/blob/bb0f54f95454352371968f76f4b3b450726b565a/src/Components/Blazor/Validation/src/ValidateComplexTypeAttribute.cs"/>
[AttributeUsage(AttributeTargets.Property, AllowMultiple = false)]
public sealed class ValidateComplexTypeAttribute : ValidationAttribute
{
/// <remarks>Based on aspnetcore 3.2 ObjectGraphDataAnnotationsValidator</remarks>
/// <seealso cref="https://github.com/dotnet/aspnetcore/issues/28640"/>
/// <seealso cref="https://www.nuget.org/packages/Microsoft.AspNetCore.Components.DataAnnotations.Validation"/>
/// <seealso cref="https://github.com/dotnet/aspnetcore/blob/bb0f54f95454352371968f76f4b3b450726b565a/src/Components/Blazor/Validation/src/ObjectGraphDataAnnotationsValidator.cs"/>
public sealed class ObjectGraphDataAnnotationsValidator : ComponentBase, IDisposable
{

@ghost
Copy link

ghost commented Dec 19, 2023

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@RichardPoes
Copy link

Classic.

@gbthakkar
Copy link

Please do it as soon as possible.

@nbiada
Copy link

nbiada commented Jan 3, 2024

I think the DataAnnotationValidator for Blazor Webassembly is integrated with ObjectGraphDataAnnotationsValidator.
In Blazor Web App Server in .net 8 I need the package and the ValidateComplex annotation, but in Webassembly I don't need it.
Any idea?

@stephajn
Copy link

stephajn commented Jan 3, 2024

This seriously needs to be migrated into the Core FX for Blazor. Just looking at the download count of the pre-release packages on NuGet.org it is absolutely clear that enough people are using it and need it.
Is there some sort of delay or consideration that we don't know about that prevents this assembly from coming out of pre-release / RC?

@ite-klass
Copy link

ite-klass commented Jan 4, 2024

Is there some sort of delay or consideration that we don't know about that prevents this assembly from coming out of pre-release / RC?

The source code file no longer exists in aspnetcore. The nuget package last version was 3.2, indicating aspnet core version 3.2. The commit (linked from the nuget package desc) is four years ago.

I tried to locate the removal commit (which may include reasoning), but after the file move to webassembly namespace I couldn't follow it anymore.

I'm sure there must have been discussion on it here too, but it's not obvious from all the "me too" and "need" and "when" comment noise.

@DjeeBay
Copy link

DjeeBay commented Feb 2, 2024

If it's going to be released, please make it compatible with netstandard 2.0.

@bcheung4589
Copy link

bcheung4589 commented Apr 4, 2024

"pranavkm commented on Dec 14, 2020"

Here we are, 4 years later, still with the need for nested validations and yet.. input validations doesnt seem to be important to the aspnetcore team?

Also love that the documentation shows to use [ValidateComplexType] and yet, where the F is it? Not in the nuget package.
https://learn.microsoft.com/en-us/aspnet/core/blazor/forms/validation?view=aspnetcore-8.0#nested-models-collection-types-and-complex-types

Or is that documentation only for the aspnetcore team? And not for us?

@stephajn
Copy link

stephajn commented Apr 4, 2024

Is there some sort of delay or consideration that we don't know about that prevents this assembly from coming out of pre-release / RC?

The source code file no longer exists in aspnetcore. The nuget package last version was 3.2, indicating aspnet core version 3.2. The commit (linked from the nuget package desc) is four years ago.

I tried to locate the removal commit (which may include reasoning), but after the file move to webassembly namespace I couldn't follow it anymore.

Even if the source code for it can't be located in the repo history, the source code is easily attainable and can and should be added back in or at least some sort of movement on this should be done because the guidance on the documentation shows that this package should be used for validating object graphs, and the built in one is not up to the task.

Couldn't the team just add this source back in or at least consider updating the guidance if this is going to be abandoned? Four years is a long time to leave developers using something marked as "experimental".

@ite-klass
Copy link

Even if the source code for it can't be located in the repo history

it's still there, on the tagged version rather than main branch IIRC

see my earlier comment for references #28640 (comment)

@bcheung4589
Copy link

bcheung4589 commented Apr 5, 2024

The fix, ridiculously simple and yet to hard to be implemented? I would love to hear the reasons this isnt implemented yet since we all know how important it is to validate user input. And so to support nesting does not seem trivial to me.

https://gist.github.com/bcheung4589/be0f762c7eeaab3655b4eb9041be4f0b#file-pfxvalidationmessage-cs-L49
Just adding line 49 made nesting work. Using FluentValidation. All the rest is just taken over from ValidationMessage.cs

The addition of L49 ensures the EditContext triggers the validation on the specified field.
This trigger will ensure "EditContext.GetValidationMessages(field)" has messages to render in case of invalid in:

foreach (var message in CurrentEditContext.GetValidationMessages(_fieldIdentifier))

@FelipeCostaGualberto
Copy link

Hello, is there news on this matter?

@FelipeCostaGualberto
Copy link

I found the latest source code of ObjectGraphDataAnnotationsValidator here: https://github.com/dotnet/aspnetcore/tree/5da314644ae882ff22fb43101d0c3d89a35c40e9/src/Components/WebAssembly/Validation/src

If you want to replicate its functionalities, you basically need these two files:
ObjectGraphDataAnnotationsValidator.cs
ValidateComplexTypeAttribute.cs

@mkArtakMSFT mkArtakMSFT modified the milestones: .NET 10 Planning, Backlog Nov 1, 2024
@stephajn
Copy link

@mkArtakMSFT why do you keep pushing this off?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
affected-medium This issue impacts approximately half of our customers area-blazor Includes: Blazor, Razor Components enhancement This issue represents an ask for new feature or an enhancement to an existing one Pillar: Complete Blazor Web Priority:2 Work that is important, but not critical for the release severity-major This label is used by an internal tool
Projects
None yet
Development

No branches or pull requests