A better Model Binder addendum

A while back, I wrote about a ModelBinder enhancement we use to do arbitrary filtering on types.  The underlying matching algorithm only matches on one type, but we like to use layer supertypes for a lot of our domain objects, so we want to use a single model binder for every Entity or Enumeration type in our system.

However, I missed an important piece in our model binding story: ModelState.  In the original implementation:

public override object BindModel(
    ControllerContext controllerContext, 
    ModelBindingContext bindingContext)
{
    foreach (var filteredModelBinder in _filteredModelBinders)
    {
        if (filteredModelBinder.IsMatch(bindingContext.ModelType))
        {
            return filteredModelBinder.BindModel(controllerContext, bindingContext);
        }
    }

    return base.BindModel(controllerContext, bindingContext);
}

I didn’t address ModelState.  It turns out that this may lead to problems later on when I use the built-in HtmlHelper methods.  These helper methods use ModelState to do things like set error message indicators, fill in values and so on.  But if one of the IFilteredModelBinder implementations binds a value but doesn’t do the ModelState piece, I start to get some weird behavior in certain scenarios.

If you start to dig pretty deeply in the DefaultModelBinder code, you’ll find that the base implementation of BindModel will set the ModelState values for certain scenarios.  In our case, we need to allow the IFilteredModelBinders to do something similar.

Our slight fix

First, we’ll need to fix the IFilteredModelBinder interface to return not just an object, but something that includes the ValueProviderResult object (used by ModelState).  Why not just do object?  Well, we can’t make an assumption on what the underlying model binder implementations need to do with a BindResult, it’s rather custom for each implementation.  So, our IFilteredModelBinder interface becomes:

public interface IFilteredModelBinder
{
    bool IsMatch(ModelBindingContext bindingContext);
    BindResult BindModel(
        ControllerContext controllerContext, 
        ModelBindingContext bindingContext);
}

That BindResult object is not in MVC, it’s something we have to define:

public class BindResult
{
    public object Value { get; private set; }
    public ValueProviderResult ValueProviderResult { get; private set; }

    public BindResult(object value, ValueProviderResult valueProviderResult)
    {
        Value = value;
        ValueProviderResult = valueProviderResult ?? 
            new ValueProviderResult(null, string.Empty, CultureInfo.CurrentCulture);
    }
}

All object adds is the ValueProviderResult, providing a default null-object implementation if the implementing filtered model binder doesn’t support ValueProviderResults.  With this in place, we need to fix our SmartBinder:

public class SmartBinder : DefaultModelBinder
{
    private readonly IFilteredModelBinder[] _filteredModelBinders;

    public SmartBinder(IFilteredModelBinder[] filteredModelBinders)
    {
        _filteredModelBinders = filteredModelBinders;
    }

    public override object BindModel(
        ControllerContext controllerContext, 
        ModelBindingContext bindingContext)
    {
        foreach (var filteredModelBinder in _filteredModelBinders)
        {
            if (filteredModelBinder.IsMatch(bindingContext))
            {
                var result = filteredModelBinder.BindModel(controllerContext, bindingContext);

                bindingContext.ModelState.SetModelValue(
                    bindingContext.ModelName, 
                    result.ValueProviderResult);

                return result.Value;
            }
        }

        return base.BindModel(controllerContext, bindingContext);
    }
}

It’s pretty much like the original, except that piece that does the SetModelValue part.  That fills in the necessary pieces for the input HtmlHelper methods to do their thing.  Interestingly, this made another problem go away, around ModelState.AddModelError.  It turns out I just missed a piece in the custom model binding code.

So just a note to custom model binder implementors – don’t forget to populate the appropriate ModelState values, especially if the results of the binding operation will be used in conjunction with the HtmlHelper extensions.