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

Data Annotations on record fails in unittest #49011

Closed
MarcoHuib opened this issue Mar 2, 2021 · 7 comments
Closed

Data Annotations on record fails in unittest #49011

MarcoHuib opened this issue Mar 2, 2021 · 7 comments

Comments

@MarcoHuib
Copy link

Currently we are setting up a new project and like to use the new records introduced in C# 9.
We encounter a problem with DataAnnotations inside the record (constructor) not being triggered during the unittest.

Now the DataAnnotation is triggered when calling the Controller, but when i try to simulate this in a unittest (see code below) it will never return any errors.

        //Unit Testing ASP.NET DataAnnotations validation
        //http://stackoverflow.com/questions/2167811/unit-testing-asp-net-dataannotations-validation
        protected static IList<ValidationResult> ValidateModel(object model)
        {
            var validationResults = new List<ValidationResult>();
            var ctx = new ValidationContext(model, null, null);
            Validator.TryValidateObject(model, ctx, validationResults, true);
            return validationResults;
        }

Currently we try different solutions but nothing works...

    public record FooRecord(string BarProperty)
    {
        [Required]
        public string BarProperty { get; init; } = BarProperty;

    }

This results in an error when i using this with mvc api.

System.InvalidOperationException: Record type '*******.FooRecord' has validation metadata defined on property 'BarProperty' that will be ignored. 'BarProperty' is a parameter in the record primary constructor and validation metadata must be associated with the constructor parameter.

As someone comment on stackoverflow to use targets in dataannotation.
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/attributes/#attribute-targets

    public record FooRecord([property: Required]string BarProperty)
    {
    }

Results in a error:
Record type '*******.FooRecord' has validation metadata defined on property 'BarProperty' that will be ignored. 'BarProperty' is a parameter in the record primary constructor and validation metadata must be associated with the constructor parameter.

The only workaround that is possible is write the complete record

    public record FooRecord
    {
        [Required]
        public string BarProperty { get; init; }

        public FooRecord([Required] string barProperty) => (BarProperty) = (barProperty);
        public void Deconstruct(out string barProperty)
        {
            barProperty = BarProperty;
        }
    }

I think this happens because the constructor of a record are params and properties at the same time.
I'm hoping if someone knows why this happens and maybe know how to solve this using the shorthand syntax:

    public record FooRecord([Required] BarProperty){ }
@antonfirsov antonfirsov transferred this issue from dotnet/core Mar 2, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.ComponentModel.DataAnnotations untriaged New issue has not been triaged by the area owner labels Mar 2, 2021
@ghost
Copy link

ghost commented Mar 2, 2021

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

Issue Details

Currently we are setting up a new project and like to use the new records introduced in C# 9.
We encounter a problem with DataAnnotations inside the record (constructor) not being triggered during the unittest.

Now the DataAnnotation is triggered when calling the Controller, but when i try to simulate this in a unittest (see code below) it will never return any errors.

        //Unit Testing ASP.NET DataAnnotations validation
        //http://stackoverflow.com/questions/2167811/unit-testing-asp-net-dataannotations-validation
        protected static IList<ValidationResult> ValidateModel(object model)
        {
            var validationResults = new List<ValidationResult>();
            var ctx = new ValidationContext(model, null, null);
            Validator.TryValidateObject(model, ctx, validationResults, true);
            return validationResults;
        }

Currently we try different solutions but nothing works...

    public record FooRecord(string BarProperty)
    {
        [Required]
        public string BarProperty { get; init; } = BarProperty;

    }

This results in an error when i using this with mvc api.

System.InvalidOperationException: Record type '*******.FooRecord' has validation metadata defined on property 'BarProperty' that will be ignored. 'BarProperty' is a parameter in the record primary constructor and validation metadata must be associated with the constructor parameter.

As someone comment on stackoverflow to use targets in dataannotation.
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/attributes/#attribute-targets

    public record FooRecord([property: Required]string BarProperty)
    {
    }

Results in a error:
Record type '*******.FooRecord' has validation metadata defined on property 'BarProperty' that will be ignored. 'BarProperty' is a parameter in the record primary constructor and validation metadata must be associated with the constructor parameter.

The only workaround that is possible is write the complete record

    public record FooRecord
    {
        [Required]
        public string BarProperty { get; init; }

        public FooRecord([Required] string barProperty) => (BarProperty) = (barProperty);
        public void Deconstruct(out string barProperty)
        {
            barProperty = BarProperty;
        }
    }

I think this happens because the constructor of a record are params and properties at the same time.
I'm hoping if someone knows why this happens and maybe know how to solve this using the shorthand syntax:

    public record FooRecord([Required] BarProperty){ }
Author: MarcoHuib
Assignees: -
Labels:

area-System.ComponentModel.DataAnnotations, untriaged

Milestone: -

@Joe4evr
Copy link
Contributor

Joe4evr commented Mar 2, 2021

Use the property target specifier:
Didn't see you already tried that.

So why doesn't the validation work if it's just on the parameter?

public record FooRecord([Required] string BarProperty){ }

@MarcoHuib
Copy link
Author

Use the property target specifier:

public record FooRecord([property: Required] BarProperty){ }

Hi,

I tried but this results to an error.

As someone comment on stackoverflow to use targets in dataannotation.
https://docs.microsoft.com/en-us/dotnet/csharp/programming-guide/concepts/attributes/#attribute-targets

    public record FooRecord([property: Required]string BarProperty)
    {
    }

Results in a error:
Record type '*******.FooRecord' has validation metadata defined on property 'BarProperty' that will be ignored. 'BarProperty' is a parameter in the record primary constructor and validation metadata must be associated with the constructor parameter.

@MarcoHuib
Copy link
Author

MarcoHuib commented Mar 2, 2021

Use the property target specifier:
Didn't see you already tried that.

So why doesn't the validation work if it's just on the parameter?

public record FooRecord([Required] string BarProperty){ }

Funny part, this works with my mvc api. but when I create a unittest it doesn't detect an error.
(All other solutions do work with this unittest)

        //Unit Testing ASP.NET DataAnnotations validation
        //http://stackoverflow.com/questions/2167811/unit-testing-asp-net-dataannotations-validation
        protected static IList<ValidationResult> ValidateModel(object model)
        {
            var validationResults = new List<ValidationResult>();
            var ctx = new ValidationContext(model, null, null);
            Validator.TryValidateObject(model, ctx, validationResults, true);
            return validationResults;
        }

@dferretti
Copy link

We have a similar use case and are trying to find a good workaround. The current plan is to just use classes instead of records for now. Here are some relevant links I can dump here:
dotnet/aspnetcore#23976
#47602
aspnet/Mvc#8606

Basically, [Required] string Bar is attached to the constructor param, and [property: Required] string Bar is attached to the generated property. Unfortunately Validator.TryValidateObject only looks at property attributes, while MVC has special case for constructor param attributes, and throws on property attributes.
From the 3rd link, it explains that the validation MVC does is not identical to Validator.TryValidateObject, and looks to expose exactly that MVC validation as an easy-to-use package - but was closed...

In the 2nd link @pranavkm says this would be a feature request to add similar support to Validator. That would be one solution. Another I wonder if MVC can be configured to not throw on property attributes. Then I could write something like

[Required][property: Required] string Bar

as a workaround. It's ugly but for my use case it is in a Source Generator so would largely be hidden.

@dferretti
Copy link

Actually after reading more into it - I don't think adding constructor attribute support will be enough. It looks like Validator.TryValidateObject does not recurse through inner property values.
I'm leaning towards if you want to manually validate an object the same way MVC automatically validates an incoming object, you have to jump through the MVC hoops and not lean on Validator.TryValidateObject hoping it will mimic it.

@ajcvickers
Copy link
Member

Support for records is not something we plan to add to the validation attributes. The bottom line here is that the validation attributes are what they are. It's old and not very good code. I would not recommend it going forward. At some point there probably needs to be a new validation library that is easier to evolve and maintain, but there are currently no concrete plans for this.

@ajcvickers ajcvickers removed the untriaged New issue has not been triaged by the area owner label Jul 13, 2021
@dotnet dotnet locked as resolved and limited conversation to collaborators Aug 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants