IValidatableObject Validate method firing when DataAnnotations fails

12,012

Considerations after comments' exchange:

The consensual and expected behavior among developers is that IValidatableObject's method Validate() is only called if no validation attributes are triggered. In short, the expected algorithm is this (taken from the previous link):

  1. Validate property-level attributes
  2. If any validators are invalid, abort validation returning the failure(s)
  3. Validate the object-level attributes
  4. If any validators are invalid, abort validation returning the failure(s)
  5. If on the desktop framework and the object implements IValidatableObject, then call its Validate method and return any failure(s)

However, using question's code, Validate is called even after [Required] triggers. This seems an obvious MVC bug. Which is reported here.

Three possible workarounds:

  1. There's a workaround here although with some stated problems with it's usage, apart from breaking the MVC expected behavior. With a few changes to avoid showing more than one error for the same field here is the code:

    viewModel
        .Validate(new ValidationContext(viewModel, null, null))
        .ToList()
        .ForEach(e => e.MemberNames.ToList().ForEach(m =>
        {
            if (ModelState[m].Errors.Count == 0)
                ModelState.AddModelError(m, e.ErrorMessage);
        }));
    
  2. Forget IValidatableObject and use only attributes. It's clean, direct, better to handle localization and best of all its reusable among all models. Just implement ValidationAttribute for each validation you want to do. You can validate the all model or particular properties, that's up to you. Apart from the attributes available by default (DataType, Regex, Required and all that stuff) there are several libraries with the most used validations. One which implements the "missing ones" is FluentValidation.

  3. Implement only IValidatableObject interface throwing away data annotations. This seems a reasonable option if it's a very particular model and it doesn't requires much validation. On most cases the developer will be doing all that regular and common validation (i.e. Required, etc.) which leads to code duplication on validations already implemented by default if attributes were used. There's also no re-usability.

Answer before comments:

First of all I've created a new project, from scratch with only the code you provided. It NEVER triggered both data annotations and Validate method at the same time.

Anyway, know this,

By design, MVC3 adds a [Required]attribute to non-nullable value types, like int, DateTime or, yes, decimal. So, even if you remove required attribute from that decimal it works just like it is one there.

This is debatable for its wrongness (or not) but its the way it's designed.

In you example:

  • 'DataAnnotation' triggers if [Required] is present and no value is given. Totally understandable from my point of view
  • 'DataAnnotation' triggers if no [Required] is present but value is non-nullable. Debatable but I tend to agree with it because if the property is non-nullable, a value must be inputted, otherwise don't show it to the user or just use a nullable decimal.

This behavior, as it seems, may be turned off with this within your Application_Start method:

DataAnnotationsModelValidatorProvider.AddImplicitRequiredAttributeForValueTypes = false;

I guess the property's name is self-explanatory.

Anyway, I don't understand why do you want to the user to input something not required and don't make that property nullable. If it's null then it is your job to check for it, if you don't wan't it to be null, before validation, within the controller.

public ActionResult Index(HomeViewModel viewModel)
{
    // Complete values that the user may have 
    // not filled (all not-required / nullables)

    if (viewModel.Decimal == null) 
    {
        viewModel.Decimal = 0m;
    }

    // Now I can validate the model

    if (ModelState.IsValid)
    {
        HomeModel.ProcessHome(viewModel);
        return RedirectToAction("Ok");
    }
}

What do you think it's wrong on this approach or shouldn't be this way?

Share:
12,012
Diego
Author by

Diego

I'm 28 years old and I started working as a developer as soon as I graduated ORT high-school technical course of studies. I've been working in software development ever since and I have acquired experience with different kind of teams and projects. I've also worked with some clients on my own as a freelancer. Since the beginning of my tech career, I've wanted to grow as a software architect and I'm proud to say I have. I've mainly worked with .NET web technologies and been in charge of different projects, which led me to becoming the architect of a very large and complex product. After getting my Systems Analyst degree I found a new passion in Information Security, so I studied an Information Security Master's degree at Universidad de Buenos Aires. Although I'm not sure I'd like to work directly at the Security area, I love applying that knowledge to development and I really think that's an underrated profile. Besides being kind of a geek and a techie, I've trained Taekwon-do for a lot of years and I enjoy playing sports like handball, tennis and squash. I also like cooking and, of course, eating!

Updated on June 08, 2022

Comments

  • Diego
    Diego about 2 years

    I've a ViewModel which has some DataAnnotations validations and then for more complex validations implements IValidatableObject and uses Validate method.

    The behavior I was expecting was this one: first all the DataAnnotations and then, only if there were no errors, the Validate method. How ever I find out that this isn't always true. My ViewModel (a demo one) has three fileds one string, one decimal and one decimal?. All the three properties have only Required attribute. For the string and the decimal? the behavior is the expected one, but for the decimal, when empty, Required validation fails (so far so good) and then executes the Validate method. If I inspect the property its value is zero.

    What is going on here? What am I missing?

    Note: I know that Required attribute is suppose to check if the value is null. So I'd expect to be told not to use Required attribute in not-nullable types (because it wont ever trigger), or, that somehow the attribute understand the POST values and note that the field wasn't filled. In the first case the attribute shouldn't trigger and the Validate method should fire. In the second case the attribute should trigger and the Validate method shouldn't fire. But my result are: the attributes triggers and the Validate method fires.

    Here is the code (nothing too special):

    Controller:

    public ActionResult Index()
    {
        return View(HomeModel.LoadHome());
    }
    
    [HttpPost]
    public ActionResult Index(HomeViewModel viewModel)
    {
        try
        {
            if (ModelState.IsValid)
            {
                HomeModel.ProcessHome(viewModel);
                return RedirectToAction("Index", "Result");
            }
        }
        catch (ApplicationException ex)
        {
            ModelState.AddModelError(string.Empty, ex.Message);
        }
        catch (Exception ex)
        {
            ModelState.AddModelError(string.Empty, "Internal error.");
        }
        return View(viewModel);
    }
    

    Model:

    public static HomeViewModel LoadHome()
    {
        HomeViewModel viewModel = new HomeViewModel();
        viewModel.String = string.Empty;
        return viewModel;
    }
    
    public static void ProcessHome(HomeViewModel viewModel)
    {
        // Not relevant code
    }
    

    ViewModel:

    public class HomeViewModel : IValidatableObject
    {
        [Required(ErrorMessage = "Required {0}")]
        [Display(Name = "string")]
        public string String { get; set; }
    
        [Required(ErrorMessage = "Required {0}")]
        [Display(Name = "decimal")]
        public decimal Decimal { get; set; }
    
        [Required(ErrorMessage = "Required {0}")]
        [Display(Name = "decimal?")]
        public decimal? DecimalNullable { get; set; }
    
        public IEnumerable<ValidationResult> Validate(ValidationContext validationContext)
        {
            yield return new ValidationResult("Error from Validate method");
        }
    }
    

    View:

    @model MVCTest1.ViewModels.HomeViewModel 
    
    @{
        Layout = "~/Views/Shared/_Layout.cshtml";
    }
    
    @using (Html.BeginForm(null, null, FormMethod.Post))
    {
        <div>
            @Html.ValidationSummary()
        </div>
        <label id="lblNombre" for="Nombre">Nombre:</label>
        @Html.TextBoxFor(m => m.Nombre)
        <label id="lblDecimal" for="Decimal">Decimal:</label>
        @Html.TextBoxFor(m => m.Decimal)
        <label id="lblDecimalNullable" for="DecimalNullable">Decimal?:</label>
        @Html.TextBoxFor(m => m.DecimalNullable)
        <button type="submit" id="aceptar">Aceptar</button>
        <button type="submit" id="superAceptar">SuperAceptar</button>
        @Html.HiddenFor(m => m.Accion)
    }
    
  • Diego
    Diego over 12 years
    I agree with you when you said its right the implicit Required to non-nullable types. My problem (that you seem not to be able to reproduce) is that the implicit (or explicit) Required attribute is triggered and then the Validate method is called. I'd expect this method never be called when some DataAnnotations validation failed (no matter if it is implicit or explicit)
  • Joao
    Joao over 12 years
    There's just one scenario where I can reproduce such behavior (and forget that AddImplicitRequiredAttributeForValueTypes exists) : when using non-nullable decimal without [Required] on top of it. It seems that the implicit required isn't counted as a DataAnnotation trigger and Validate is called.
  • Diego
    Diego over 12 years
    Yes but not only. I can also reproduce it with the Required attribute. Such as my code is, you can't?
  • Joao
    Joao over 12 years
    Ups, you're right, that scenario also triggers both attributes and Validate. The normal (or at least wanted behavior) is Validate being called after attributes validation. If that's not happening I would call it a bug.
  • Joao
    Joao over 12 years
    That behavior is consensual among developers. Check here or even here on a post by a Microsoft ASP.Net employee.
  • Joao
    Joao over 12 years
    He even specifies the validation process's chronology. (1) Validate property-level attributes, (2) If any validators are invalid, abort validation returning the failures, (3) Validate the object-level attributes, (4) If any validators are invalid, abort validation returning the failures and finally (5) If on the desktop framework and the object implements IValidatableObject, then call its Validate method and return any failures
  • Diego
    Diego over 12 years
    I still coudn't see the links but I should assume this as a bug. Which would be the best workaround? Right now I only have one and I don't like it: stackoverflow.com/questions/6431478/…
  • Joao
    Joao over 12 years
    Well, there's no access to the validation errors dictionary on IValidatableObject so it's not possible to check if the model is already marked with errors before Validate().
  • Joao
    Joao over 12 years
    If you don't want to mess with the controllers code like showed on that workaround (something which I totally understand!), why don't forget IValidatableObject and use only attributes? It's clean, direct, better to handle localization and best of all its reusable among all models. Just implement ValidationAttribute for each validation you want to do. You can validate the all model or particular properties, that's up to you.
  • Joao
    Joao over 12 years
    Apart from the attributes available by default (DataType. Regex, Required and all that stuff) you've several libraries with the most used validations. One which implements the "missing ones" is foolproof.
  • Diego
    Diego over 12 years
    if I have to pick between only IValidatableObject OR only attributes I'll choose the first one. I can explain this decision because I think it is much cleaner when the validation involves more than one field.
  • Diego
    Diego over 12 years
    Please update your answer for future readers. Its clearly the one to be accepted! Thanks a lot.
  • Diego
    Diego over 12 years
    Please update your answer with all this comments and I'll accept it.
  • Joao
    Joao over 12 years
    Sorry, I've been busy :) Look, if you've the time leave a new Issue here attaching your code sample. It would be nice to hear something from their side.
  • Shiva
    Shiva about 7 years
    Thanks @Joao... EPIC Answer! I was wondering why the Validate was not firing. This answer clarified it and I confirmed it in my code :)