Naming is important

Just stumbled over a “bug” in .NET 4.0 DataAnnotations/Validator that was quite easy to “work around”, but still took us a while to figure out.

We want to do simple structural server-side validations.

Data Annotations

/* haven’t installed my Live Writer beautifier-plugin yet */

public class SomeCommand : Command {

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

    [Required] public sting City { get; set; }

    [Range(0,99)] public string Age { get; set; }

}

The base class uses System.ComponentModel.DataAnnotations.Validator to run the validations in this simple Method:

private IEnumerable<ValidationResult> AutoValidation()
{
    var results = new List<ValidationResult>();
    Validator.TryValidateObject(this, new ValidationContext(this, null, null), results);
    return results;
}

Problem

It only validated the fields marked as required. The RangeAttribute was ignored.

After a while we had a look at the .NET-Framework-Sources using Reflector, and found this:

if (validateAllProperties)
{
    source.AddRange(GetValidationErrors(pair.Value, pair.Key, propertyValidationAttributes, breakOnFirstError));
}
else
{
    RequiredAttribute validationAttribute = propertyValidationAttributes.FirstOrDefault<ValidationAttribute>(delegate (ValidationAttribute a) {
        return (a is RequiredAttribute);
    }) as RequiredAttribute;
    if (validationAttribute != null)
   {
       // validate the RequiredAttribute

validateAllProperties in this case means: do not only run Required-Validations.

Solution

Actually a different override of Validator.TryValidateObject has a flag named validateAllProperties. But since it did already validate more than one property, I assumed true was the default.

Lesson I learned: do not assume anything!

Lesson for MS: Take time finding the right name! Do refactor!

I filed a bug here: https://connect.microsoft.com/VisualStudio/feedback/details/605635/missleading-parametername-validateallproperties-in-validator-try-validate-componentemodel-dataannotations

Advertisements

2 thoughts on “Naming is important

  1. Refactoring your public API is nearly impossible when doing framework design. You have to design your API with extreme care. Otherwise you’ll end up with… exactly this.

    I must say that Microsoft has done a tremendous job with the API of the .NET framework .I mean, if you’ve done any work in PHP and the absolute madness of those libraries behind it and you’ll start to appreciate the .NET framework.

Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s