Form validation with MVC Framework Preview 5

The latest release of the MVC Framework, Preview 5, has some nice additions for doing form based validation. Scott Guthrie has an in-depth blog post here describing these features, so please read that if you haven't already.

Very briefly the validation framework has four elements:

1. Model binders based on the IModelBinder interface. You can override the default binders by setting properties on the static ModelBinders class.
2. New UpdateModel methods on Controller that bind a given model to the Request.Form. The model binders are also used to bind request form values to action parameters.
3. A ModelStateDictionary that maintains the results of a binding operation. This is maintained in the ViewData.
4. Updated HtmlHelper input control renderers (like TextBox)  and a new HtmlHelper ValidationMessage extension method. These work together to show validation messages on the form by reading the ModelStateDictionary that's contained in the ViewData.

On the whole it's a nicely thought out framework. I especially like the ability to slot in your own implementations of IModelBinder. But there are other things I don't like so much, especially the way the UpdateModel methods on the controller work. In fact I think that the base controller class is getting far too god-like, with too many concerns over and above executing action methods. Having all these controller methods makes actions hard to test and the whole framework less 'plugable'.

In his post, Scott shows how to implement a validation rules engine that you can plug in to your domain entities. I really don't like this approach, it relies on the developer always remembering to call a particular validation method after setting properties. That or an over intimate relationship between the entity and the ORM.

I like to put validation in my domain entity's property setters. It just seems the most natural and obvious place for them.I've blogged about this before, but today I want to show how it can work very nicely with most of the preview 5 framework.

Have a look at this property of my domain entity:

public virtual string Code
{
    get { return code; }
    set
    {
        code = value.Label("Code").IsRequired().Value;
    }
}

IsRequired is an extension method that raises a ValidationException if value is null or empty. The ValidationException it raises has a nice message "You must enter a value for Code". Now if we simply use the UpdateModel method out-of-the-box like this:

[AcceptVerbs("POST")]
public ActionResult Edit(int id, FormCollection form)
{
    var centre = centreRepository.GetById(id);
    try
    {
        UpdateModel(centre, new[]{"Code", "NetscapeName"});
        centreRepository.SubmitChanges();
        return RedirectToAction("Edit", new { id });
    }
    catch (Exception)
    {
        return RenderEditView(centre);
    }
}

We get this message on the page when we forget to enter a Code:

Not the nice message that I was expecting. The reason for this is that UpdateModel has a try-catch block that catches Exception and inserts its own message: "The value '<input value>' is invalid for property '<property name>'" whatever the exception may have been. That's not really very useful. Here's a section of the code from the MVC Framework source (I think I'm allowed to put this on my blog):

foreach (var property in properties) {
    string fieldName = property.Key;
    PropertyDescriptor propDescriptor = property.Value;
    IModelBinder converter = ModelBinders.GetBinder(propDescriptor.PropertyType);
    object convertedValue = converter.GetValue(ControllerContext, fieldName, propDescriptor.PropertyType, ViewData.ModelState);
    if (convertedValue != null) {
        try {
            propDescriptor.SetValue(model, convertedValue);
        }
        catch {
            // want to use the current culture since this message is potentially displayed to the end user
            string message = String.Format(CultureInfo.CurrentCulture, MvcResources.Common_ValueNotValidForProperty,
                convertedValue, propDescriptor.Name);
            string attemptedValue = Convert.ToString(convertedValue, CultureInfo.CurrentCulture);
            ViewData.ModelState.AddModelError(fieldName, attemptedValue, message);
        }
    }
}

Having a blanket catch like that around the property setter is not a good idea. What if some unexpected exception happened? I wouldn't know about it, I'd just assume that there was some type conversion error. Also there's no way to get your own exception message into the ModelState at this stage. Scott's blog post shows a work-around where you populate the ModelState at a later stage in the controller, but I want my framework to do that for me.

OK, so I don't like the UpdateModel method on the controller, I'd much rather have my validator injected by my IoC container. To this end I've implemented my own ValidatingBinder:

using System;
using System.Collections.Generic;
using System.Collections.Specialized;
using System.Web.Mvc;
using Suteki.Common.Extensions;
namespace Suteki.Common.Validation
{
    public class ValidatingBinder : IValidatingBinder
    {
        private readonly List<IBindProperties> propertyBinders;
        public ValidatingBinder() : this(new IBindProperties[0])
        {
        }
        public ValidatingBinder(params IBindProperties[] propertyBinders)
        {
            this.propertyBinders = new List<IBindProperties>(propertyBinders);
        }
        public List<IBindProperties> PropertyBinders
        {
            get { return propertyBinders; }
        }
        public virtual void UpdateFrom(object target, NameValueCollection values)
        {
            UpdateFrom(target, values, new ModelStateDictionary(), null);
        }
        public virtual void UpdateFrom(object target, NameValueCollection values, ModelStateDictionary modelStateDictionary)
        {
            UpdateFrom(target, values, modelStateDictionary, null);
        }
        public virtual void UpdateFrom(object target, NameValueCollection values, string objectPrefix)
        {
            UpdateFrom(target, values, new ModelStateDictionary(), objectPrefix);
        }
        public virtual void UpdateFrom(
            object target, 
            NameValueCollection values, 
            ModelStateDictionary modelStateDictionary, 
            string objectPrefix)
        {
            UpdateFrom(new BindingContext(target, values, objectPrefix, modelStateDictionary));
        }
        public virtual void UpdateFrom(BindingContext bindingContext)
        {
            foreach (var property in bindingContext.Target.GetType().GetProperties())
            {
                try
                {
                    foreach (var binder in propertyBinders)
                    {
                        binder.Bind(property, bindingContext);
                    }
                }
                catch (Exception exception)
                {
                    if (exception.InnerException is FormatException ||
                        exception.InnerException is IndexOutOfRangeException)
                    {
                        bindingContext.AddModelError(property.Name, bindingContext.AttemptedValue, "Invalid value for {0}".With(property.Name));
                    }
                    else if (exception.InnerException is ValidationException)
                    {
                        bindingContext.AddModelError(property.Name, bindingContext.AttemptedValue, exception.InnerException);
                    }
                    else
                    {
                        throw;
                    }
                }
            }
            if (!bindingContext.ModelStateDictionary.IsValid)
            {
                throw new ValidationException("Bind Failed. See ModelStateDictionary for errors");
            }
        }
        /// <summary>
        /// IModelBinder.GetValue
        /// </summary>
        public virtual object GetValue(
            ControllerContext controllerContext, 
            string modelName, 
            Type modelType, 
            ModelStateDictionary modelState)
        {
            if (controllerContext == null)
            {
                throw new ArgumentNullException("controllerContext");
            }
            if (String.IsNullOrEmpty(modelName))
            {
                throw new ArgumentException("Cannot be null or empty", "modelName");
            }
            if (modelType == null)
            {
                throw new ArgumentNullException("modelType");
            }
            if (IsBasicType(modelType))
            {
                return new DefaultModelBinder().GetValue(controllerContext, modelName, modelType, modelState);
            }
            var instance = Activator.CreateInstance(modelType);
            var request = controllerContext.HttpContext.Request;
            var form = request.RequestType == "POST" ? request.Form : request.QueryString;
            UpdateFrom(instance, form); 
            return instance;
        }
        private static bool IsBasicType(Type type)
        {
            return (type.IsPrimitive ||
                type.IsEnum ||
                type == typeof(decimal) ||
                type == typeof(Guid) ||
                type == typeof(DateTime) ||
                type == typeof(string));
        }
    }
}

There are a few things to note about this class. The first is that the actual property binding itself is delegated to a list of property binders that implement the IBindProperties interface. This means I can chain as many specialized property binders that I like and have the IoC container inject them into the ValidatingBinder. All the information needed by the binding operation; the target entity, the form values and the ModelStateDictionary are contained in a BindingContext instance that also knows how to get a particular property value from the form values. I've also implemented IModelBinder.GetValue so that it will work with action property binding as well.

The most important thing for this discussion is the try-catch block in the UpdateFrom method. Note that I look for specific exception types. If I can find a ValidationException I pass that to the ModelStateDictionary which means that the message I want will be shown on the form.

OK, so here's my action method, but now it's using the ValidatingBinder:

[AcceptVerbs("POST")]
public ActionResult Edit(int id, FormCollection form)
{
    var centre = centreRepository.GetById(id);
    try
    {
        validatingBinder.UpdateFrom(centre, form, ViewData.ModelState);
        centreRepository.SubmitChanges();
        return RedirectToAction("Edit", new { id });
    }
    catch (ValidationException)
    {
        return RenderEditView(centre);
    }
}

When we attempt an update without a code we now get this nice validation exception:

I hope this post doesn't come across as too much of a criticism of the validation infrastructure in preview 5.  Most of it I like. I have to have a specialized binder in any case for binding NHibernate entities when I'm updating an object graph, so it's very satisfying that the new control renderers can be made to work with it without too much sweat.