Question Custom Validation Attributes in .NET Core

[Mateycho]

Hi guys, I am really struggling the previous week to find out why is there a difference when I pass the error property through the constructor of my custom validation attribute or when I pass it explicitly. I have been looking through the source code so much that my eyes started hurting, hah. When I pass it through the constructor on the client side I get the following error: [de-CH: isValidEmail], which might be since we have many languages on our application, but when I pass the same string explicitly it returns the correctly formated error.

Can someone explain why we have this behavior? Also, what would be the best practice with validation attributes to override bool isValid and FormatErrorMessage methods or something other, since I don't want to pass the ErrorMessage in the ViewModel to pollute the code there? Thanks!

Currently, I am doing things like in the picture and it works fine, but I am just speechless why doesn't the error message work when passed from the constructor? Could it be because it expects some formating, or culture definition before passing and the ErrorMessage explicitly is like the default one, which doesn't need such modifications to be displayed correctly? Can someone help?

 

[jmcilhinney]

Please don't post pictures of code. Post all text as text, formatted appropriately. There are dedicated formatting options for code. Only post a picture in addition to the text if it adds value, which this doesn't. We can't copy code from a picture and they are harder to manage on a small device.

 

[jmcilhinney]

Please provide specific examples of code that demonstrates the issue you're experiencing and explain exactly what you expect to see and what you actually see.

 

[Mateycho]

Please provide specific examples of code that demonstrates the issue you're experiencing and explain exactly what you expect to see and what you actually see.

Ok here are more details about the problem, that is the actual code which I want to understand where is the difference between that code and the second one:

[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field, AllowMultiple = false)]
public class IsValidEmailAttribute : ValidationAttribute
{
    private string _pattern = RegexConst.REGEX_VALID_EMAIL;

    public IsValidEmailAttribute() : base(StaticText.ApplicantRegisterViewModel_Email_RegularExpression)
    {
    }
    public override bool IsValid(object value)
    {
        // Validate Email
        if (value == null || !DataValidation.IsValidStringRegEx(_pattern, value.ToString()))
        {
            return false;
        }

        return true;
    }
}

This is the second the one which I have stopped on now, but there actually shouldn't be any difference, since they both pass the values to the errorMessage property behind the scenes:

[AttributeUsage(AttributeTargets.Property | AttributeTargets.Field, AllowMultiple = false)]
public class IsValidEmailAttribute : ValidationAttribute
{
    private string _pattern = RegexConst.REGEX_VALID_EMAIL;

    public IsValidEmailAttribute()
    {
        ErrorMessage = StaticText.ApplicantRegisterViewModel_Email_RegularExpression
    }
    public override bool IsValid(object value)
    {
        // Validate Email
        if (value == null || !DataValidation.IsValidStringRegEx(_pattern, value.ToString()))
        {
            return false;
        }

        return true;
    }
}

Although there shouldn't be any differences From the first one I get the following validation error: [de-CH: IsValidEmail], which is like a pointer to the place I am getting the error from and not the actual error, like in the case with the second one!

Please help me understand where is the problem, or where exactly is the difference between those two approaches as the internet doesn't't have much to say about them!

 

[Skydiver]

In your screenshot on the first post, you are overriding FormatErrorMessage(), but when we asked you to repost the code as text you changed over to showing calling the base constructor with an error message vs. setting the ErrorMessage property and saying that setting the ErrorMessage property is not working.

I'm surprised that setting the ErrorMessage property even compiles for you. It seems to be missing a semicolon at the end of line 8.

 

[Mateycho]

In your screenshot on the first post, you are overriding FormatErrorMessage(), but when we asked you to repost the code as text you changed over to showing calling the base constructor with an error message vs. setting the ErrorMessage property and saying that setting the ErrorMessage property is not working.

I'm surprised that setting the ErrorMessage property even compiles for you. It seems to be missing a semicolon at the end of line 8.

In the comment before the picture I explain well enough that this is neither the approach that gives me the error, nor the one I used. This is the current one which I am using.

 

[Skydiver]

Ah! I missed that. Small phone and must have scrolled too much.

 

[Skydiver]

The following code works both ways for me:
.NET:

using System;
using System.ComponentModel.DataAnnotations;
using System.Linq;

static class ObjectExtensions
{
    public static bool HasDigits(this object? value)
        => value != null && value.ToString().ToCharArray().Any(ch => char.IsDigit(ch));
}

class MyNoDigitsSetErrorMessageAttribute : ValidationAttribute
{
    public MyNoDigitsSetErrorMessageAttribute()
        => ErrorMessage = "No numbers allowed dude!";

    public override bool IsValid(object? value)
        => !value.HasDigits();
}

class MyNoDigitsUseConstructorAttribute : ValidationAttribute
{
    public MyNoDigitsUseConstructorAttribute()
        : base("Numbers are for the weak.")
    {
    }

    public override bool IsValid(object? value)
        => !value.HasDigits();
}

class Program
{
    public static void Main()
    {
        var setErrorMessage = new MyNoDigitsSetErrorMessageAttribute();
        var useConstructor = new MyNoDigitsUseConstructorAttribute();
        var testStrings = new [] { "abcd", "ab12cd", "5" };

        foreach(var testString in testStrings)
        {
            TestValidation(setErrorMessage, testString);
            TestValidation(useConstructor, testString);
        }

        void TestValidation(ValidationAttribute attribute, string value)
        {
            var message = attribute.IsValid(value)
                            ? "valid"
                            : attribute.FormatErrorMessage(value);
            Console.WriteLine($"{value}: {message}");
        }
    }
}

.NET Framework:

using System;
using System.ComponentModel.DataAnnotations;
using System.Linq;

static class ObjectExtensions
{
    public static bool HasDigits(this object value)
        => value != null && value.ToString().ToCharArray().Any(ch => char.IsDigit(ch));
}

class MyNoDigitsSetErrorMessageAttribute : ValidationAttribute
{
    public MyNoDigitsSetErrorMessageAttribute()
        => ErrorMessage = "No numbers allowed dude!";

    public override bool IsValid(object value)
        => !value.HasDigits();
}

class MyNoDigitsUseConstructorAttribute : ValidationAttribute
{
    public MyNoDigitsUseConstructorAttribute()
        : base("Numbers are for the weak.")
    {
    }

    public override bool IsValid(object value)
        => !value.HasDigits();
}

class Program
{
    public static void Main()
    {
        var setErrorMessage = new MyNoDigitsSetErrorMessageAttribute();
        var useConstructor = new MyNoDigitsUseConstructorAttribute();
        var testStrings = new[] { "abcd", "ab12cd", "5" };

        foreach (var testString in testStrings)
        {
            TestValidation(setErrorMessage, testString);
            TestValidation(useConstructor, testString);
        }

        void TestValidation(ValidationAttribute attribute, string value)
        {
            var message = attribute.IsValid(value)
                            ? "valid"
                            : attribute.FormatErrorMessage(value);
            Console.WriteLine($"{value}: {message}");
        }
    }
}

There is likely something else at play in your scenario.

 

[Mateycho]

Could it be because I have different cultures in my application? And something goes wrong behind the scenes in that case? But I have tried the same thing as you -> A hardcoded string and it still doesn't give me the correct error on the clientSide

 

[Skydiver]

My reading of the code is that if ErrorMessage is set, then no attempts are made to read resources to get the error message string.

Hmmm... You mentioned "clientSide". Are you sure that your updated binaries are being pushed to the client side?

I would recommend coming up with a minimal reproducable scenario. For example, above I tried to take out any extra layers of the attribute being invoked by some other layers of code and just called the attribute code directly.

Try calling your attribute directly. If that provides correct results, then next try to directly invoke the data annotations validation directly without relying on any UI -- eg. just a Console app. Does that still work correctly? If so, then move on to does the data annotation validation work correctly when running server side? If that still works correctly, then next involve the client-server data annotation validation.

 

[jmcilhinney]

For future reference, there's no need to quote an entire post when you're just replying to the previous one. You're just making the thread longer and harder to read for no good reason. Only quote a previous post if you need to specifically indicate what you're replying to and, even then, only quote the part of the post that you're specifically replying to. If someone provides a long code snippet, there's generally no good reason to quote that code. You might do so if you want to draw attention to a specific line but then you can quote just that line. Quoting a comment from the post will let everyone know what post you're responding to and they can then go to that post to see the code if necessary. There was no need for you to quote my early post asking for code examples. Just say that you're providing code examples.

 

[Skydiver]

Okay, using the Data Annotations validator instead of calling the validation attributes directly still works:

using System;
using System.Collections.Generic;
using System.ComponentModel.DataAnnotations;
using System.Linq;

static class ObjectExtensions
{
    public static bool HasDigits(this object? value)
        => value != null && value.ToString().ToCharArray().Any(ch => char.IsDigit(ch));
}

class MyNoDigitsSetErrorMessageAttribute : ValidationAttribute
{
    public MyNoDigitsSetErrorMessageAttribute()
        => ErrorMessage = "No numbers allowed dude!";

    public override bool IsValid(object? value)
        => !value.HasDigits();
}

class MyNoDigitsUseConstructorAttribute : ValidationAttribute
{
    public MyNoDigitsUseConstructorAttribute()
        : base("Numbers are for the weak.")
    {
    }

    public override bool IsValid(object? value)
        => !value.HasDigits();
}

class MyEntity
{
    [MyNoDigitsSetErrorMessage]
    public string SetErrorMessage { get; set; }

    [MyNoDigitsUseConstructor]
    public string UseConstructor { get; set; }

    public override string ToString()
        => $"{{ SetErrorMessage = '{SetErrorMessage}', UseConstructor = '{UseConstructor}' }}";
}

class Program
{
    public static void Main()
    {
        var testStrings = new[] { "abcd", "ab12cd", "5" };

        foreach (var testString in testStrings)
        {
            var entity = new MyEntity()
            {
                SetErrorMessage = testString,
                UseConstructor = testString
            };

            var results = new List<ValidationResult>();
            if (Validator.TryValidateObject(entity, new ValidationContext(entity), results, validateAllProperties: true))
            {
                Console.WriteLine($"{entity} is valid.");
            }
            else
            {
                Console.WriteLine($"{entity} is invalid.");
                foreach (var result in results)
                {
                    var members = String.Join(", ", result.MemberNames);
                    Console.WriteLine($"{members}: {result.ErrorMessage}");
                }
            }
        }
    }
}

 

[Mateycho]

My reading of the code is that if ErrorMessage is set, then no attempts are made to read resources to get the error message string.

To be honest I think the problem might be connected with how the code under the hood behaves when you call the constructor with no arguments, those are the three constructors of the decompiled code of the ValidatioAttribute:

protected ValidationAttribute()
    : this(() => DataAnnotationsResources.ValidationAttribute_ValidationError)
{
}

protected ValidationAttribute(string errorMessage)
    : this(() => errorMessage)
{
}

protected ValidationAttribute(Func<string> errorMessageAccessor)
{
    _errorMessageResourceAccessor = errorMessageAccessor;
}

Even though I don't pass anything it still uses this default error from the DataAnnotationsResources. And when it goes in the

private void SetupResourceAccessor()
{
    if (_errorMessageResourceAccessor != null)
    {
        return;
    }

    string localErrorMessage = ErrorMessage;
    bool flag = !string.IsNullOrEmpty(_errorMessageResourceName);
    bool flag2 = !string.IsNullOrEmpty(_errorMessage);
    bool flag3 = _errorMessageResourceType != null;
    bool flag4 = !string.IsNullOrEmpty(_defaultErrorMessage);
    if ((flag && flag2) || !(flag || flag2 || flag4))
    {
        throw new InvalidOperationException(DataAnnotationsResources.ValidationAttribute_Cannot_Set_ErrorMessage_And_Resource);
    }

    if (flag3 != flag)
    {
        throw new InvalidOperationException(DataAnnotationsResources.ValidationAttribute_NeedBothResourceTypeAndResourceName);
    }

    if (flag)
    {
        SetResourceAccessorByPropertyLookup();
        return;
    }

    _errorMessageResourceAccessor = () => localErrorMessage;
}

It goes into the first IF and returns, meaning the ErrorMessage is never used. And theoretically, if we look at the code never should there be a case where it goes after the if, since all of the constructors refer to the third one where the _errorMessageResourceAccessor is set, so this accessor will be always set. And there goes my question. How then internally is the Error message set so that when I set it explicitly I still can use it although the first constructor is called? And of course my first question is why doesn't it work when I do it from the constructor, which looking at your research skydiver I see it is connected with some other part of my code, maybe some additional layer

 

[Skydiver]

Kudos for disassembling just to make sure about exactly what you are running. I was lazy and was just looking at:

Source Browser

or

Reference Source

It goes into the first IF and returns, meaning the ErrorMessage is never used.

Not quite. Notice these two bits of code:

public virtual string FormatErrorMessage(string name) {
    return String.Format(CultureInfo.CurrentCulture, this.ErrorMessageString, name);
}

and

protected string ErrorMessageString {
    get {
        this.SetupResourceAccessor();
        return this._errorMessageResourceAccessor();
    }
}

All the various flavors of calling the validation will eventually call FormatErrorMessage(). This is why your code in your first post works because you just overrode this implementation. The default implementation as you can see accesses the ErrorMessageString. That property's getter always calls SetupResourceAccessor() which is the code you were discussing in post #13.

Your analysis that the first if statement will just return immediately because _errorMessageResourceAccessor is not null is correct. But notice the next line of the getter which invokes that function referred to by _errorMessageResourceAccessor.

Since that instance variable is set by this constructor ValidationAttribute(Func<string> errorMessageAccessor) which in turn was called by ValidationAttribute(string errorMessage), then the error message passed in via constructor is used. But you are correct the public ErrorMessage property is not used.

Now on the other hand, if the constructor without parameters is used, but ErrorMessage is set, notice the implementation of the setter:

public string? ErrorMessage
{
    // If _errorMessage is not set, return the default. This is done to preserve
    // behavior prior to the fix where ErrorMessage showed the non-null message to use.
    get => _errorMessage ?? _defaultErrorMessage;
    set
    {
        _errorMessage = value;
        _errorMessageResourceAccessor = null;
        CustomErrorMessageSet = true;
 
        // Explicitly setting ErrorMessage also sets DefaultErrorMessage if null.
        // This prevents subsequent read of ErrorMessage from returning default.
        if (value == null)
        {
            _defaultErrorMessage = null;
        }
    }
}

See that it sets _errorMessageResourceAccessor to null. So when instance variable is null, then the first if statement of SetupResourceAccessor() will not be true, and will not return immediately. Instead, it will try to try to setup _errorMessageResourceAccessor to be a delegate that returns the captured value of ErrorMessage after some validation checks for internal consistency. Notice that the code looks at ErrorMessageResourceName vs ErrorMessage, and will throw an exception if both are set.

Based your descrliption of the problem, the code seems to be behaving as if ErrorMessageResourceName was set instead of ErrorMessage.

 

[Mateycho]

Notice that the code looks at ErrorMessageResourceName vs ErrorMessage, and will throw an exception if both are set.

Sorry I didn't get you here, but the deep dive that you made is awesome, good job! Can you sum up the information that you found? Don't get me wrong I have read everything you wrote. My question is do we have some clues now as to why the ValidationAttribute might behave differently when errorMessage is set explicitly or when passed through the constructor? Thanks for your efforts!