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

[Netfx bug] CustomValidationAttribute doesn't properly implement RequiresValidationContext #21100

Closed
hughbe opened this issue Apr 14, 2017 · 4 comments
Labels
area-System.ComponentModel.DataAnnotations bug disabled-test The test is disabled in source code against the issue
Milestone

Comments

@hughbe
Copy link
Contributor

hughbe commented Apr 14, 2017

Requested by @ajcvickers. We should mark this as netfx-port-consider and close this issue.

See dotnet/corefx#5203, which fixed this.

public class CustomValidator
{
    public static ValidationResult CorrectValidationMethodOneArg(object o) => ValidationResult.Success;
    public static ValidationResult CorrectValidationMethodOneArgStronglyTyped(string s) => ValidationResult.Success;
    public static ValidationResult CorrectValidationMethodTwoArgs(object o, ValidationContext context) => ValidationResult.Success;
    public static ValidationResult CorrectValidationMethodTwoArgsStronglyTyped(TestClass tc, ValidationContext context) => ValidationResult.Success;
}


[Theory]
[InlineData(nameof(CustomValidator.CorrectValidationMethodOneArg), false)]
[InlineData(nameof(CustomValidator.CorrectValidationMethodOneArgStronglyTyped), false)]
[InlineData(nameof(CustomValidator.CorrectValidationMethodTwoArgs), true)]
[InlineData(nameof(CustomValidator.CorrectValidationMethodTwoArgsStronglyTyped), true)]
public static void RequiresValidationContext_Get_ReturnsExpected(string method, bool expected)
{
    CustomValidationAttribute attribute = new CustomValidationAttribute(typeof(CustomValidator), method);

    // The full .NET framework has a bug where CustomValidationAttribute doesn't
    // validate the context. See https://github.com/dotnet/corefx/pull/5203.
    if (PlatformDetection.IsFullFramework)
    {
        Assert.False(attribute.RequiresValidationContext);
    }
    else
    {
        Assert.Equal(expected, attribute.RequiresValidationContext);
    }
}
@karelz
Copy link
Member

karelz commented Apr 14, 2017

The PR dotnet/corefx#5203 was already marked as netfx-port-consider. Here's more information, so following @hughbe's instructions. (Thanks @hughbe!)

@karelz karelz closed this as completed Apr 14, 2017
@divega
Copy link
Contributor

divega commented Apr 16, 2017

@karelz could you please confirm that what the process is for netfx-port-consider? Is including the label on an already merged PR usually enough?

@karelz
Copy link
Member

karelz commented Apr 16, 2017

@AlexGhiondea created the tool and will likely oversee future .NET Framework porting effort (at least from auditing perspective) - I let him answer.
I am inclined to think that PR with the label should be enough ...

cc @danmosemsft

@AlexGhiondea
Copy link
Contributor

@divega yes -- the tool should pick up the issue even if closed.

@msftgits msftgits transferred this issue from dotnet/corefx Jan 31, 2020
@msftgits msftgits added this to the 1.0.0-rtm milestone Jan 31, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-System.ComponentModel.DataAnnotations bug disabled-test The test is disabled in source code against the issue
Projects
None yet
Development

No branches or pull requests

5 participants