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

MetadataTypeAttribute doesn't influence DataAnnotations validation result #46678

Closed
RichardBr opened this issue Apr 14, 2020 · 45 comments · Fixed by #51772
Closed

MetadataTypeAttribute doesn't influence DataAnnotations validation result #46678

RichardBr opened this issue Apr 14, 2020 · 45 comments · Fixed by #51772

Comments

@RichardBr
Copy link

RichardBr commented Apr 14, 2020

Update by @SteveSandersonMS: please see the comment #46678 (comment) for a description of the underlying issue/question.

Please ignore the parts of this thread that are specific to Blazor. That's just how the issue was originally reported, but it looks like the underlying concern is about DataAnnotations itself.


Describe the bug

When I use the MetadataType attribute (from System.ComponentModel.DataAnnotations) on the class for the purpose of validation I get the following error in the console window
WASM: System.TypeLoadException: Could not resolve type with token 01000027 from typeref (expected class 'System.ComponentModel.DataAnnotations.MetadataTypeAttribute' in assembly 'System.ComponentModel.Annotations, Version=4.3.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a')

Greater details of the error are shown in this file...[console screenshot - validation error](https://user-images.githubusercontent.com/5938313/79199691-42595400-7e2d-11ea-95bc-3c2248d8b60e.png

Please note my sample application does works when I put all validation attributes into main the class.

For clarity:
This approach works...

public partial class PersonViewModel
{
    [Required(ErrorMessage = "First name is required.")]
    public string Fname { get; set; }
}

This approach fails...

public partial class PersonViewModel
{
    public string Fname { get; set; }
}

[MetadataType(typeof(PersonViewModelMetaData))]
public partial class PersonViewModel
{
}

public class PersonViewModelMetaData
{
    (ErrorMessage = "First name is required.")]
    public string Fname { get; set; }
}

To Reproduce

Please run the attached sample blazor application. To see the error please click the navigation link labelled "validation - failure". On the page, just simply press the submit button to see the error.
BlazorAppValidation.zip

Further technical details

  • Include the output of dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   3.1.300-preview-015048
 Commit:    13f19b4682

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18363
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.1.300-preview-015048\

Host (useful for support):
  Version: 3.1.3
  Commit:  4a9f85e9f8

.NET Core SDKs installed:
  3.1.101 [C:\Program Files\dotnet\sdk]
  3.1.201 [C:\Program Files\dotnet\sdk]
  3.1.300-preview-015048 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

@javiercn
Copy link
Member

@RichardBr thanks for contacting us.

This is not a feature that we currently support AFAIK.

/cc: @SteveSandersonMS @pranavkm

@nssidhu
Copy link

nssidhu commented Apr 23, 2020

This is much needed as in case of scaff folding it just delete and recreates the class again, we would have to re-write everything again. please consider to implement it, need for real life project

@mkArtakMSFT
Copy link
Member

We've moved this issue to the Backlog milestone. This means that it is not going to happen for the coming release. We will reassess the backlog following the current release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.

@wmgdev
Copy link

wmgdev commented Jun 10, 2020

Agree, It would be really nice if MetadataType worked.
It would allow us to add data annotations for validation to Models generated using Scaffold-DbContext

@MattSturtivant
Copy link

Just wanted to add a +1 for this - while we can setup a task to mix-in our data annotations after scaffolding for now, I feel this is a regression from MVC.

@Gambero81
Copy link

this is a must have feature, especially in projects with automatic class generations like Entity Framework, to allow adding additional validation or business rules.

Hope it will be implemented as soon as possible

@IanKemp
Copy link

IanKemp commented Sep 7, 2020

We've moved this issue to the Backlog milestone. This means that it is not going to happen for the coming release. We will reassess the backlog following the current release and consider this item at that time. However, keep in mind that there are many other high priority features with which it will be competing for resources.

Those "high-priority" features won't matter if big corporates decline to use Blazor because its model-binding support sucks.

@navidzehi
Copy link

This is really an important issue. One of the main advantages of Blazor versus frameworks like Angular is the rapid development due to capability of reusing the 'server code' for validation or generation of models in the client. @mkArtakMSFT

@SteveSandersonMS
Copy link
Member

I've just been checking this, and I don't think this issue is Blazor-specific. I think it's a more general point that System.ComponentModel.DataAnnotations.MetadataTypeAttribute doesn't actually do anything to influence the DataAnnotations validation result.

To repro my findings, create a new .NET Core Console Application (so that's neither Blazor nor WebAssembly), containing:

    [MetadataType(typeof(PersonMetadata))]
    public class Person
    {
        public string Name { get; set; }
    }

    public class PersonMetadata
    {
        [Required]
        public string Name { get; set; }
    }

    class Program
    {
        static void Main(string[] args)
        {
            // Some people say you should do this, but it doesn't seem to make any difference
            TypeDescriptor.AddProviderTransparent(
                new AssociatedMetadataTypeTypeDescriptionProvider(typeof(Person), typeof(PersonMetadata)),
                typeof(Person));

            var person = new Person();
            var results = new List<ValidationResult>();
            var isValid = Validator.TryValidateObject(person, new ValidationContext(person), results);
            Console.WriteLine($"isValid: {isValid}");
        }
    }

I would have expected this to say isValid: False but actually it says isValid: True. I can only get it to respect the [Required] attribute by putting it directly onto Person, not PersonMetadata. Also notice that this has nothing to do with partial classes (I'm not using them above).

@pranavkm Do we have any contacts who work on DataAnnotations? This seems like a weirdness that a lot of people have complained about all over StackOverflow etc. I can't tell whether it used to work in the past (e.g. on .NET Framework) since some people say it does if you use TypeDescriptor.AddProviderTransparent, but it certainly doesn't appear to work now.

@Gambero81
Copy link

news on this issue?

@DiegoVenancioVieira
Copy link

news on this issue?

Nothing, there are many other priorities. This is just one more in the backlog list. Waiting too

@SteveSandersonMS SteveSandersonMS transferred this issue from dotnet/aspnetcore Jan 7, 2021
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.ComponentModel.DataAnnotations untriaged New issue has not been triaged by the area owner labels Jan 7, 2021
@ghost
Copy link

ghost commented Jan 7, 2021

Tagging subscribers to this area: @ajcvickers
See info in area-owners.md if you want to be subscribed.

Issue Details

Describe the bug

When I use the MetadataType attribute (from System.ComponentModel.DataAnnotations) on the class for the purpose of validation I get the following error in the console window
WASM: System.TypeLoadException: Could not resolve type with token 01000027 from typeref (expected class 'System.ComponentModel.DataAnnotations.MetadataTypeAttribute' in assembly 'System.ComponentModel.Annotations, Version=4.3.1.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a')

Greater details of the error are shown in this file...[console screenshot - validation error](https://user-images.githubusercontent.com/5938313/79199691-42595400-7e2d-11ea-95bc-3c2248d8b60e.png

Please note my sample application does works when I put all validation attributes into main the class.

For clarity:
This approach works...

public partial class PersonViewModel
{
    [Required(ErrorMessage = "First name is required.")]
    public string Fname { get; set; }
}

This approach fails...

public partial class PersonViewModel
{
    public string Fname { get; set; }
}

[MetadataType(typeof(PersonViewModelMetaData))]
public partial class PersonViewModel
{
}

public class PersonViewModelMetaData
{
    (ErrorMessage = "First name is required.")]
    public string Fname { get; set; }
}

To Reproduce

Please run the attached sample blazor application. To see the error please click the navigation link labelled "validation - failure". On the page, just simply press the submit button to see the error.
BlazorAppValidation.zip

Further technical details

  • Include the output of dotnet --info
.NET Core SDK (reflecting any global.json):
 Version:   3.1.300-preview-015048
 Commit:    13f19b4682

Runtime Environment:
 OS Name:     Windows
 OS Version:  10.0.18363
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\3.1.300-preview-015048\

Host (useful for support):
  Version: 3.1.3
  Commit:  4a9f85e9f8

.NET Core SDKs installed:
  3.1.101 [C:\Program Files\dotnet\sdk]
  3.1.201 [C:\Program Files\dotnet\sdk]
  3.1.300-preview-015048 [C:\Program Files\dotnet\sdk]

.NET Core runtimes installed:
  Microsoft.AspNetCore.All 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 2.1.16 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.2 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 3.1.3 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET Core runtimes or SDKs:
  https://aka.ms/dotnet-download

Author: RichardBr
Assignees: -
Labels:

area-System.ComponentModel.DataAnnotations, untriaged

Milestone: -

@SteveSandersonMS SteveSandersonMS changed the title Blazor validation doesnt work when using MetadataType attribute on classs MetadataTypeAttribute doesn't influence DataAnnotations validation result Jan 7, 2021
@julior77
Copy link

Any news on this issue?
As mentioned this is really important for teams using Scaffold-DbContext.

Any alternatives as a short term?

@DiegoVenancioVieira
Copy link

Scaffold-DbContext
Any news on this issue?
As mentioned this is really important for teams using Scaffold-DbContext.
Any alternatives as a short term?

No

@Xaeco
Copy link
Contributor

Xaeco commented Jan 25, 2021

Did major work migrating my project to Blazor and didn't see this one coming.
Now I'm stuck.
Any workarounds would be appreciated.

@navidzehi
Copy link

@Xaeco Maybe this can help you for now C# new source generator

@DiegoVenancioVieira
Copy link

@Xaeco Maybe this can help you for now C# new source generator

@navidzehi Can you tell us HOW? Which part of video ?

@navidzehi
Copy link

@DiegoVenancioVieira well,I haven't try it myself but I think it's possible to do the following.
You scaffold or manually create your model classes normally :

[MetadataType(typeof(PersonMetaData))]
public partial class Person
{
   public int Id {get;set;}
   public string FirstName{get;set;}
   public string LastName{get;set;}
}

then you may also have the following classes :

// in another partial file :
public partial class Person
{
 [NoMap]
  public string FullName => FirstName+" "+LastName;
}

public class PersonMetaData
{
   public object Id {get;set;}
 
   [Required(ErrorMessage = "FirstName is required.")]
   public object FirstName{get;set;}
   [Required(ErrorMessage = "LastName is required.")]
   public object LastName{get;set;}
 }

Now in a way that is mentioned in the video you have generate the following model and instead of using the Person model you use the Person_Generated(with your own naming convensions) in your whole application :


public partial class Person_Generated 
{
   public int Id {get;set;}
   [Required(ErrorMessage = "FirstName is required.")]
   public string FirstName{get;set;}
   [Required(ErrorMessage = "LastName is required.")]
   public string LastName{get;set;}
}

you also need to add the following Partial class manually (or maybe you can manage to generate it too for an existing project):

public partial class Person_Generated 
{
[NoMap]
  public string FullName => FirstName+" "+LastName;
}

another option is also to include the FullName in the first Person_Generated.cs partial file.

@Xaeco
Copy link
Contributor

Xaeco commented Jan 26, 2021

@Xaeco Maybe this can help you for now C# new source generator

Thanks for your time. What I'm attempting to do is decorate classes that have been auto generated by .proto file compilation so that the DataAnnotationsValidator in Blazor has something to work with.

As per your example I'd need to write a source generator to make a new set of classes based on the auto-generated ones and weave in the data annotations.

@IanShaughnessyPG
Copy link

I am also very interested in this feature. We would like to decorate classes generated from .proto files as well and use them with the DataAnnotationsValidator component.

I dug into the code a bit and it looks like the .NET 5 version of System.ComponentModel.DataAnnotations.ValidationAttributeStore is using System.Reflection to get the list of attributes, whereas the .NET Framework version is using TypeDescriptor and PropertyDescriptor.

@Xaeco
Copy link
Contributor

Xaeco commented Feb 13, 2021

I'm going to have a go at fixing it

@michaelr-sportsbet
Copy link

This seems to be generally the problem with decorating any autogenerated class with attributes. I am experiencing a similar issue with trying to add XmlElement and XmlAttribute to classes generated from a proto file that need to be deserialized from an xml payload. The MetadataType attribute seems to have no effect whatsoever.

I also tried to use MetadataType on a simple class and it seems to be just completely ignored, so it's not something to do with autogenerated files or protobuf, it just seems to not work at all even in the simplest scenario.

(I am using .net standard 2.1 and System.ComponentModel.Annotations nuGet version 5.0.0)

@Xaeco
Copy link
Contributor

Xaeco commented Feb 24, 2021

I created a small test project to investigate the DataAnnotations behaviour.
Attached zip has two projects with identical code. A .Net Framework project called DN and a .Net Core project called Core.
TestMetadataTypeAttribute.zip

Some findings:
To get the Metadata type to work in .Net Framework 4.5+ you need to ensure two things

  • When using Validator.TryValidateObject() you need to set the validateAllProperties parameter to True. Otherwise only the Required attribute gets checked.
  • You need to ensure you add a provider transport as shown in previous posts
TypeDescriptor.AddProviderTransparent(
                new AssociatedMetadataTypeTypeDescriptionProvider(typeof(Person), typeof(PersonMetadata)),
                typeof(Person));

You don't need to use

[MetadataType(typeof(PersonMetadata))]

As it makes no difference to .Net Framework 4.5+

The Core project doesn't produce the expected results.

I went through the .Net Core 5 code, specifically through the System.ComponentModel.Annotations solution. I couldn't find any direct references to MetadataTypeAttribute or AssociatedMetadataType in the context of the TryValidateObject code.

I will keep digging.

@Xaeco
Copy link
Contributor

Xaeco commented Mar 1, 2021

So I have coded two working solutions.

Solution 1

  • produces the same results as .Net 4.8
  • requires TypeDescriptor.AddProviderTransparent() method.
  • doesn't use the MetadataType attribute.
  • could potentially be made so that any declaration of the MetadataType attribute would auto register the TypeDescriptor.AddProviderTransparent() method negating the need for code.

Solution 2

  • produces the same results as .Net 4.8
  • uses the MetadataType attribute
  • doesn't use the TypeDescriptor.AddProviderTransparent() method.

Both solutions require adding a couple of methods to the System.ComponentModel.Annotations.Validator class. This resides in .Net Standard 2.1. I'm guessing any fixes would come out in a .NET 6 beta and not to any older versions?

So I'm hoping for some assistance here on which implementation to use. Like to hear everyone's vote?

@wolfgangschneider
Copy link

I prefer Solution 2 , Its the same as old silverlight WCF RIA services used.

@georgeemr
Copy link

georgeemr commented Mar 6, 2021

Solution 2 for sure!.
I'm working on a project on Blazor, and I came through this issue with my current scaffolded models. Xaeco please release the second solution this could help us a lot!

@Xaeco
Copy link
Contributor

Xaeco commented Mar 6, 2021

Thank you to all who have commented.
It looks like solution 2 is preferred.
I hope to get a pull request in today.

@Xaeco
Copy link
Contributor

Xaeco commented Mar 22, 2021

@SteveSandersonMS I have two working solutions for this issue.

Solution 1. Is to replace two functions with their DN4.8 equivalents. They use TypeDescriptor, which as far as I can tell, is being avoided.

Solution 2. I wrote functions that work but when I do a full build I get an IL Linker error which I can't suppress and I'm not sure I should.

I'd appreciate talking to someone re the best way forward.

@rahul7720
Copy link

Thanks for the effort guys.
Solution 2 would be the neater solution. As mentioned by others this issue has a considerable impacts.
Any Idea, when can we expect the fix?

@SteveSandersonMS
Copy link
Member

@Xaeco Thanks for looking into this.

@eerhardt Do you know who would be a good person on the runtime team to direct this to for triaging/feedback?

@pranavkm
Copy link
Contributor

FYI @ajcvickers

@eerhardt
Copy link
Member

The listed owners of the area-System.ComponentModel.DataAnnotations area are @lajones @ajcvickers.

Solution 2. I wrote functions that work but when I do a full build I get an IL Linker error which I can't suppress and I'm not sure I should.

@Xaeco - can you try rebasing your change on the latest main? I merged #49901 last week, which resolved the existing ILLink warnings in that library. If you are still seeing warnings, can you send them to me, along with your change? I can assist in getting them resolved.

@Xaeco
Copy link
Contributor

Xaeco commented Apr 1, 2021

Excellent. I will give it a go and let you know. Thank you> The listed owners of the area-System.ComponentModel.DataAnnotations area are @lajones @ajcvickers.

Solution 2. I wrote functions that work but when I do a full build I get an IL Linker error which I can't suppress and I'm not sure I should.

@Xaeco - can you try rebasing your change on the latest main? I merged #49901 last week, which resolved the existing ILLink warnings in that library. If you are still seeing warnings, can you send them to me, along with your change? I can assist in getting them resolved.

@Xaeco
Copy link
Contributor

Xaeco commented May 23, 2021

The listed owners of the area-System.ComponentModel.DataAnnotations area are @lajones @ajcvickers.

Solution 2. I wrote functions that work but when I do a full build I get an IL Linker error which I can't suppress and I'm not sure I should.

@Xaeco - can you try rebasing your change on the latest main? I merged #49901 last week, which resolved the existing ILLink warnings in that library. If you are still seeing warnings, can you send them to me, along with your change? I can assist in getting them resolved.

I've had a PR in for the fix for 29 days now but haven't had anyone look at it yet. Not sure if I've missed part of the process or everyone is really busy and this is too low priority. Any assistance would be appreciated. Thanks.

@hdv212
Copy link

hdv212 commented May 26, 2021

Hi all!
What's is the best practice for this situation?
Is there any alternative approach?
Thanks in advance

@rahul7720
Copy link

rahul7720 commented May 27, 2021

@Xaeco Thanks for your contribution. I'm waiting for this fix for a long time.
I'm not sure about the process. Is there any alternative way to publish the NuGet package or to distribute the dlls?

Will this be released under DotNet 5.0 or DotNet 6.x preview?

@hdv212
Copy link

hdv212 commented May 27, 2021

Hi again.
I'm looking for a best way to accomplish form validation via extending my entities (via dataAnnotantions or someThing else) when using model first, for more than 1 year!
Please give an article or reference to do this via blazor best practice.

@Xaeco
Copy link
Contributor

Xaeco commented May 28, 2021

@Xaeco Thanks for your contribution. I'm waiting for this fix for a long time.
I'm not sure about the process. Is there any alternative way to publish the NuGet package or to distribute the dlls?

Will this be released under DotNet 5.0 or DotNet 6.x preview?

Hi. At the moment its in the DotNet 6.x preview codebase. Hopefully it gets approved soon. Meanwhile I'll look into the process to backport it to DotNet 5.0 if that's at all possible.

Currently I am testing my fix locally by incorporating the updated DotNet classes (with a few references changed) into my Blazor project and using a custom validator. The custom validator is required because Blazor's validator is using DotNet 6 Preview internally and that doesn't have my code in it. This works for DotNet 5 & 6.

I'm not sure what the rules are on sharing modified versions of the DotNet files. @eerhardt can you advise?

@eerhardt
Copy link
Member

I'm not sure what the rules are on sharing modified versions of the DotNet files. @eerhardt can you advise?

Until the change is merged into main, my recommendation would be to pull the change locally and build the file locally.

@rahul7720
Copy link

Hi Guys,

Any updates on this?

@Xaeco
Copy link
Contributor

Xaeco commented Jul 8, 2021

Still waiting on my pull request to be reviewed. No further progress.

@ajcvickers ajcvickers added this to the 6.0.0 milestone Jul 13, 2021
@ajcvickers ajcvickers removed the untriaged New issue has not been triaged by the area owner label Jul 13, 2021
@Xaeco
Copy link
Contributor

Xaeco commented Jul 25, 2021

Update. Option 1 has been approved. Not sure if it will land in a .NET 6 preview or the final release.

@Xaeco
Copy link
Contributor

Xaeco commented Jul 25, 2021

@ajcvickers any chance this fix could be back-ported to .NET 5?

If so what is the process?

@rahul7720
Copy link

@Xaeco Any chance if you can build an unofficial NuGet package / Dll and release it for testing purposes?

@bricelam bricelam added the in-pr There is an active PR which will close this issue when it is merged label Jul 27, 2021
@bricelam
Copy link
Contributor

See also (and upvote) dotnet/efcore#14544

@ghost ghost closed this as completed in #51772 Jul 28, 2021
ghost pushed a commit that referenced this issue Jul 28, 2021
* MetadataType attribute class properties are now validated.

Fix #46678

* Clean up PR #51772

Co-authored-by: Brice Lambson <brice@bricelam.net>
@ghost ghost removed the in-pr There is an active PR which will close this issue when it is merged label Jul 28, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Aug 27, 2021
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.