Pages

Monday, March 18, 2013

Which ASP.NET MVC validation strategy should we use?

At least as long as programming has existed, people have tried to be tricky.

I'm sure we have all tried to architecture something tricky only to have to revert our project and sheepishly go with a simpler solution.

Whenever I look at something that isn't a tricky optimization, it's almost always a tricky way to waste cpu cycles.

The lure of trickyness we'll be looking at is validation in ASP.NET MVC.  Let's look at 3 methods of validation.

The bad way (using model objects)


Unless you want to get hacked in the same fashion github did last year, don't do the following.  You leave yourself open to getting model injected if you do this; only new MVC users will try using model objects bound to a view.  The initial lure might be that you can do this without creating an extra ViewModel.


It's not an extensible way to write code anyway so luckily you have an excuse to not be lazy.  The second you need to use the same domain object on multiple pages for even slightly different purposes, this pattern falls apart and subsequent pages will have trouble accommodating new functionality.

Add a field like this and now we have a security problem because model binding is blind and will greedily bind anything it can.
Actually the above example is prone to being insecure out of the box.  If you only wanted to let the user update their email and password, you'd have to specifically guard the Username and Id (and any new fields as I point out above).

It also shows how unclear the above code is--you may have assumed I was taking the Id and Username from a submitted field even though this was for a profile editing controller.  It's even less readable.


The tricky way (using centralized validation close to your model)


Okay, fine we'll use ViewModels, but we're going to be tricky.  We're going to keep all of our validation inside our domain objects.  When someone goes to input some data, we'll map to our domain objects, check the validation rules and then bubble the results up.  It's genius.

Actually it's bad.  For several reasons.  Exceptions to rules will be painful.

Let's not say things we don't mean and have to take them back


It also doesn't take advantage of the model state of MVC which I find to be quite nice and allows you to check errors separately.


  • Once for easy validations like max length and whether a field is required, 
  • Again for dynamic validations that involve costly database operations.


Bundling those two operations isn't good.  Without a simple way to perform client-side checks, that's what you'll be doing (I would just bundle them on the server if you have jQuery validate on the client for the sake of saving a few lines of code in each controller).  .

I don't care what horrid path Visual Studio Magazine (scroll to 'Integrating annotations') is trying to send you down.  Don't do the tricky way.  It's cute little examples like this have people being tricky.  But then you feed tricky after midnight and bam...code gremlins.  They even seem to recommend 'The bad way'.

This pattern could work in a fully back-end scenario, but this next pattern is just too durable, efficient and fast for anything else to be used for ASP.NET MVC development.

The smart way (also the easiest way -- using ViewModels and validation attributes)



A good example.  Simple and gets created in the default MVC4 project template.


But author, creating a view model AND validation attributes for each form is more typing.  How is it the easiest way?  Because ordering your thoughts is hard.  Not being able to trigger your validation on the client, is hard.  Having to mess with your DbContext just validate user input (noise++ to your codebase) is hard.  Having to make an exception to what you thought were your domain rules becomes a hack (I'll remove this one validation rule..)

Let me be clear about that last point.  Adding something and then removing it later down the pipeline before you do any real work is a HUGE anti-pattern.  I don't know this anti-pattern by a name, but this is one of the biggest mistakes I see in code.  It's unclear, it's a hack, it's inefficient, it shouldn't be done.


Here is the article to read that will explain the best practices of the ViewModel pattern.   (Although the author doesn't mention that your HTML will be rendered with jQuery validate baked into it which is a big plus)


It's worth noting that this works great with ajax heavy sites.  I almost exclusively use ajax to submit forms and working with data annotation attributes is still preferred.  Data annotation validators will cover any  non-dynamic validations you need.  The jQuery validate attributes will be rendered in your HTML which you can easily call out to yourself (a huge benefit over the previous example--an easy way to make super responsive pages by having automatic client side validation.)

The ViewModel pattern solves so many issues.  It's not completely D.R.Y., though.

Or is it?

If you have two pages that allow modification of a profile (lets say, registration and profile updating) you will have to copy and paste your name length validator attribute to both view models.  I would say that attributes are data and not code (we've been blurring the line lately.. look at expressions) and that the code is still D.R.Y.

Conclusion

Most MVC developers already know, and practice, everything I've said above.  So why would I spend my time writing about it?   Because why not examine the other avenues and possible architectures even if they end up being poor choices?  If you ever felt like maybe you were missing something or you were wasting time when writing a bunch of ViewModels and repeating attributes on them, well, you should feel a bit more justified after reading this.  It's the standard--keep doing it.

Also, hasn't everyone tried conquering validation with a tricky pattern at some point?  I think the answer is probably yes.  But at the end of the day, being as specific as you can and failing as soon as you can (Client passes -> server side checks passes -> dynamic server side checks pass -> okay go), is always going to be king and it does not facilitate tricky patterns.

This post may leave you thinking about unit testing as much as possible because there are some maintainability considerations with validations attributes on ViewModels even if this even if it is the best way to write your MVC application.  What do you think?

5 comments:

  1. I've had good experiences using FluentValidation (http://fluentvalidation.codeplex.com) to validate ViewModels. It centralises your validation logic, lets you write separate client- and server-side validators for validation of varying complexity and renders your simpler server-side validation rules on the client as JavaScript. I'd recommend it if validation is more complex than the out-of-the-box attributes can handle easily.

    ReplyDelete
  2. Man, you saved 3 weeks of thoughts... I don't like to use attributes for validations, and I knew that's the pattern, so I was looking for someone to prouve me that's the better way.... Now I can do my attributes based validation without care this is not dry! Thank you so much.

    ReplyDelete
  3. "I would say that attributes are data and not code (we've been blurring the line lately.. look at expressions) and that the code is still D.R.Y." This is just not true.

    ReplyDelete
  4. Following is my model

    public partial class CustomerViewModel
    {
    public CustomerViewModel()
    {
    this.ServiceRequests = new HashSet();
    }

    public int CustomerId { get; set; }
    [Required]
    public string FullName { get; set; }
    [Required,DataType(DataType.EmailAddress)]
    public string Email { get; set; }
    [Required]
    public string UserPassword { get; set; }
    [Required]
    public string MobileNo { get; set; }
    [Required]
    public string AccountFrom { get; set; }
    public bool Active { get; set; }
    public System.DateTime Created { get; set; }
    public virtual ICollection ServiceRequests { get; set; }
    }


    Can we validate this with `ModelState.IsValid`

    Like If i want to validate multiple model in a single function. So, can we do that?

    ReplyDelete