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.

Resolving arrays with Windsor

Here's a class that takes an array of ISubThing as a constructor parameter:

public class Thing
{
    readonly List<ISubThing> subThings = new List<ISubThing>();
    public Thing(params ISubThing[] subThings)
    {
        this.subThings.AddRange(subThings);
    }
    public List<ISubThing> SubThings
    {
        get { return subThings; }
    }
}

Now can you guess what happens if we resolve this using MicroKernel? I naively thought that all my registered ISubThing services would get passed to the constructor, but in fact you get a handler exception saying that the container cannot find anything to supply the subThings property:

[Test, ExpectedException(typeof(HandlerException))]
public void DoesNotResolveArraysByDefault()
{
    var kernel = new DefaultKernel();
    kernel.Register(
        Component.For<Thing>(),
        Component.For<ISubThing>().ImplementedBy<First>(),
        Component.For<ISubThing>().ImplementedBy<Second>(),
        Component.For<ISubThing>().ImplementedBy<Third>()
        );
    var thing = kernel.Resolve<Thing>();
}

Hammett explains the problem here (check the comments) and shows an ArrayResolver that can be added to the kernel to enable the behaviour I was expecting. Now this test works:

[Test]
public void ShouldResolveArrayOfDependencies()
{
    var kernel = new DefaultKernel();
    kernel.Resolver.AddSubResolver(new ArrayResolver(kernel));
    kernel.Register(
        Component.For<Thing>(),
        Component.For<ISubThing>().ImplementedBy<First>(),
        Component.For<ISubThing>().ImplementedBy<Second>(),
        Component.For<ISubThing>().ImplementedBy<Third>()
        );
    var thing = kernel.Resolve<Thing>();
    Assert.That(thing.SubThings.Count, Is.EqualTo(3));
    Assert.That(thing.SubThings[0], Is.InstanceOfType(typeof(First)));
    Assert.That(thing.SubThings[1], Is.InstanceOfType(typeof(Second)));
    Assert.That(thing.SubThings[2], Is.InstanceOfType(typeof(Third)));
}

As Hammett explains in his post, this is a dangerous thing to add to your container, because the implementation he gives doesn't check for circular dependencies. Say we have this class which implements ISubThing:

public class Circular : ISubThing
{
    public Thing Thing { get; private set; }
    public Circular(Thing thing)
    {
        this.Thing = thing;
    }
}

As you can see it takes a Thing in its constructor, so we'd expect a circular reference exception to be raised by the container. But this test shows that doesn't happen:

[Test]
public void DoesNotDiscoverCircularDependencies()
{
    var kernel = new DefaultKernel();
    kernel.Resolver.AddSubResolver(new ArrayResolver(kernel));
    // a circular reference exception should be thrown here
    kernel.Register(
        Component.For<Thing>(),
        Component.For<ISubThing>().ImplementedBy<Circular>()
        );
    // this crashes the test framework!
    // var thing = kernel.Resolve<Thing>();
}

Instead, if you un-comment the last line the test framework crashes.

Andrey Shchekin has a great series of posts comparing different IoC containers:

http://blog.ashmind.com/index.php/2008/09/08/comparing-net-di-ioc-frameworks-part-2/

It's interesting that only StructureMap actually resolves array dependencies out of the box. It's such a nice feature with endless applications that I would have expected it to be more common. Interestingly MEF (which is trying very hard not to be seen as an IoC container) does this as one its core competencies. but then being able to discover a collection of components that supply a particular service is almost an essential requirement of its stated role as a plug-in framework.

ALT.NET UK (un)conference

I had a lot of fun yesterday at the ALT.NET UK (un)conference. Ian, Alan and Ben do a great job organizing it and they managed to raise a lot more sponsorship money this time which meant that they could hire a larger venue, Conway Hall.

Friday night was spent suggesting sessions. There was some kind of dance thing going on in the main hall and it was difficult to hear what was going on at times but there was still a huge range of ideas put forward. Later we retired to the pub where I had some great conversations. I especially enjoyed hearing Sebastien Lambla talking about his open rasta RESTfull web development framework. I've heard him presenting about it before, but maybe it needed a couple of pints of Czech larger for it to make sense. I'm awaiting its release with some anticipation now.

Saturday morning kicked off with a 'park bench'. I'm not sure what the subject was since I spent a little too long enjoying the hotel breakfast and complementary FT. BTW good choice of hotel Ben! I think it was something like 'what does ALT.NET mean?' There was a suggestion that we needed a list of principles somewhat like the Agile movement, but I have to agree with Alan here; I think it would be a bad idea. I didn't get to the bench, but to me ALT.NET is really simple, it means building a community around .NET development that is not controlled by Microsoft. You don't need ALT.Java because that market is diverse and competitive enough that no single vendor is perceived as the single source of tools and guidance. But that is exactly the case with .NET development. The majority of .NET shops simply look to Microsoft for both. ALT.NET provides a convenient label for the .NET community to coalesce ideas around. This is good for .NET development in general, but also good for Microsoft itself.

The first session I attended in the morning was covering ORM use. We had a great discussion of the pros and cons of NHibernate vs Linq-to-SQL vs EF. I ranted a good deal about how any ORM in the .NET space had to have a LINQ provider. We also had a good discussion around repository patterns, where LINQ fits into the specification pattern and DTOs. We stayed in the same room to talk about Multi-tenanting. There were three or four people present who were actively involved in developing multi-tenanted applications so this was a very useful discussion. It mostly revolved around database sharding vs multiple-database patterns and security concerns.

During lunch we had a very interesting and wide ranging chat about IoC containers that then became a discussion on the horrors of sharepoint development.

Ian Cooper lead a very good session on Domain Driven Development in the afternoon. I've heard Ian talk about DDD several times now, but I always seem to learn something new. I would love to see a project that he'd worked on.

This brings me to a suggestion for future ALT.NET conferences. There are some excellent conversations but I do think that it would help if more people brought laptops and we had some projectors available where we could demonstrate real code. Talking about code is very difficult without having examples in front of you. I don't mean that we should be giving presentations, but that at least we should all come prepared to show off something of what we're doing. Of course this is problematic with the commercial code that most of us work on, but being able to fire up Visual Studio (or notepad Peter :) and show rather than tell would be a huge advantage. So Ian, Alan and Ben, if you want to know what to do with the sponsorship money next time: projectors!

Today's WTF

This a real email that a colleague received just now:

"Many thanks for your regular updates I have now spoken to XXX (who was very charming).  He has confirmed our suspicion that <WTF system> in its current state is unable to handle student names that are reserved SQL keywords such as Min, Max etc" 

Thanks to Iain for matching cartoon:

http://xkcd.com/327/

MVC Framework Validation

I really enjoy Stephen Walther's blog. It's a fantastic mine of information on the MVC Framework. Recently he's been talking about Validation: here and here. He describes using validation attributes on his entities and then providing a validation framework that reads the entities and checks form post values against them. OK, that's my one-sentence summary, it's really quite sophisticated and you should read his posts because if you want to take that kind of approach his solution is well worked out.

I don't like it though. Call me old-fashioned, but I really like to have validation expressed in the domain entity itself. It seems a more DDD friendly way of doing things. Property setters, or constructor arguments, should throw exceptions if the values passed to them fail validation rules. Entities should enforce business rules rather than relying on convention to make them work. With Stephen's method it's easy to set an incorrect value on an entity. You have to explicitly invoke his framework in order for the attributes to be evaluated.

Here's an example from Suteki Shop.  Please ignore the LINQ-to-SQL nastiness, but here's a partial class for my Category entity. In the OnNameChanging partial method (that's triggered when the Name property is set) I'm using my validation extensions that I described here. If the value is not present (null or empty) a ValidationException is raised. You can't set Name to an empty or null string from anywhere in the application without this being checked.

using Suteki.Common;
using Suteki.Common.Validation;

namespace Suteki.Shop
{
    public partial class Category : IOrderable, IActivatable
    {
        partial void OnNameChanging(string value)
        {
            value.Label("Name").IsRequired();
        }

        public bool HasProducts
        {
            get
            {
                return Products.Count > 0;
            }
        }
    }
}

A common concern with this method is that the validation rules get triggered when an entity is retrieved from the database. Fortunately LINQ-to-SQL sets the backing field rather than the property setter so this doesn't happen. Any decent ORM will allow you to do the same thing.

The next issue is binding. You need a way of gathering any validation exceptions and presenting them all back to the user. I do this with a custom ValidatingBinder. The Category controller Update action looks like this:

public ActionResult Update(int categoryId)
{
    Category category = null;
    if (categoryId == 0)
    {
        category = new Category();
    }
    else
    {
        category = categoryRepository.GetById(categoryId);
    }

    try
    {
        ValidatingBinder.UpdateFrom(category, Request.Form);
    }
    catch (ValidationException validationException)
    {
        return View("Edit", EditViewData.WithCategory(category)
            .WithErrorMessage(validationException.Message));
    }

    if (categoryId == 0)
    {
        categoryRepository.InsertOnSubmit(category);
    }

    categoryRepository.SubmitChanges();

    return View("Edit", EditViewData.WithCategory(category).WithMessage("The category has been saved"));
}

The ValidatingBinder itself is mostly taken from the excellent MVCContrib. The main thing it does differently is collecting any ValidationExceptions raised from property setters and bundling them all into an uber ValidationException that is raised back to the controller.

I've recently updated ValidatingBinder to implement the new  IModelBinder interface from Preview 5. Very simple to do, and I'll write more about this soon. Here's the ValidatingBinder code:

using System;
using System.Collections.Specialized;
using System.Reflection;
using System.ComponentModel;
using System.Text;
using System.Data.Linq.Mapping;
using Suteki.Common.Extensions;

namespace Suteki.Common.Validation
{
    public class ValidatingBinder
    {
        public static void UpdateFrom(object target, NameValueCollection values)
        {
            UpdateFrom(target, values, null);
        }

        public static void UpdateFrom(object target, NameValueCollection values, string objectPrefix)
        {
            var targetType = target.GetType();
            var typeName = targetType.Name;

            var exceptionMessage = new StringBuilder();

            foreach (var property in targetType.GetProperties())
            {
                var propertyName = property.Name;
                if (!string.IsNullOrEmpty(objectPrefix))
                {
                    propertyName = objectPrefix + "." + property.Name;
                }
                if (values[propertyName] == null)
                {
                    propertyName = typeName + "." + property.Name;
                }
                if (values[propertyName] == null)
                {
                    propertyName = typeName + "_" + property.Name;
                }
                if (values[propertyName] != null)
                {
                    var converter = TypeDescriptor.GetConverter(property.PropertyType);
                    var stringValue = values[propertyName];
                    if (!converter.CanConvertFrom(typeof(string)))
                    {
                        throw new FormatException("No type converter available for type: " + property.PropertyType);
                    }
                    try
                    {
                        var value = converter.ConvertFrom(stringValue);
                        property.SetValue(target, value, null);
                    }
                    catch (Exception exception)
                    {
                        if (exception.InnerException is FormatException ||
                            exception.InnerException is IndexOutOfRangeException)
                        {
                            exceptionMessage.AppendFormat("'{0}' is not a valid value for {1}<br />", stringValue, property.Name);
                        }
                        else if (exception.InnerException is ValidationException)
                        {
                            exceptionMessage.AppendFormat("{0}<br />", exception.InnerException.Message);
                        }
                        else
                        {
                            throw;
                        }
                    }
                }
                else
                {
                    // boolean values like checkboxes don't appear unless checked, so set false by default
                    if (property.PropertyType == typeof(bool) && property.HasAttribute(typeof(ColumnAttribute)))
                    {
                        property.SetValue(target, false, null);
                    }
                }
            }
            if (exceptionMessage.Length > 0)
            {
                throw new ValidationException(exceptionMessage.ToString());
            }
        }
    }
}

You can find all this code in Suteki Shop as usual.

Managed Extensibility Framework: Why?

Sometimes... no often... I'm quite dumb. I've been skimming through posts about the MEF thinking that people were talking about the Entity Framework (Microsoft Entity Framework :). I only realised that it was something new today. So what is it. Well it looks like an IoC container, but the spin is that it's different from other IoC containers because it's primarily aimed at providing an extensibility model; a common way for applications and frameworks to load types from dropped-in assemblies. This is from the CodePlex MEF site:

"MEF presents a simple solution for the runtime extensibility problem. Until now, any application that wanted to support a plugin model needed to create its own infrastructure from scratch. Those plug-ins would often be application-specific and could not be reused across multiple implementations.

MEF provides a standard way for the host application to expose itself and consume external extensions. Extensions, by their nature, can be reused amongst different applications. However, an extension could still be implemented in a way that is application-specific. Extensions themselves can depend on one another and MEF will make sure they are wired together in the correct order (another thing you won't have to worry about).

MEF offers a set of discovery approaches for your application to locate and load available extensions.

MEF allows tagging extensions with additional metadata which facilitates rich querying and filtering"

The thing is, although they're selling it as a plug-in framework, it does look very much like any other IoC container,  so it's odd that they're making the plug-in pitch when they could be providing a generic IoC framework. There doesn't seem to be anything about it which precludes using it in the general IoC role, except that they say it's not optimised for that.

So it begs the question: are the MEF team suggesting that we support two IoC frameworks in our applications? One for plug-in extensibility, the other to support internal component based architecture?

It seems to me that it would not be very hard to provide an IoC container that can successfully cover both these requirements. I haven't tried it, but I imagine it would be quite simple to provide a facility for Windsor to load assemblies from a file location, for example.

This one is going to run and run...

What's up with Linq to NHibernate?

With my current clients I'm building an MVC Framework application using NHibernate. I love NHibernate. Like all the best frameworks it just works for most common scenarios. I'm a new user, but I've been able to get up to speed with it very quickly. The power and flexibility of the mapping options make it easy to use with the legacy database that we're lumbered with. But the best thing about it though, is that it's built from the ground up for Domain Driven Development. This means it's possible to build a finely grained domain model unencumbered by data access concerns.

Seriously, if you're building .NET enterprise level business software and you haven't considered using NHibernate (or a competing ORM) you missing out on a huge productivity win.

Any ORM targeting .NET has to support Linq. Here the NHibernate story's not so good at the moment. The coding phenomena that is Ayende kicked off the first Linq to NHibernate implementation a while back. It was originally part of his Rhino Tools project and layered the Linq implementation on top of the NHibernate criteria API. This has now been moved to the NHibernate contrib project. The source is here:

http://sourceforge.net/projects/nhcontrib/

I've been using this in my current project, it works but only with the most straightforward queries. Anything tricky tends to fail and I've tended to go through a process of trial and error to find the right patterns. The problem seems to be mostly around the criteria API. It doesn't support a lot of the features that Linq requires, especially nested queries.

Recently a new NHibernate.Linq project has been started in the NHibernate trunk that takes a different approach. The authors are building SQL directly from the expression tree which obviously gives a lot more flexibility. The intention is to also to convert HQL to an expression tree thus having a single uniform query translation.

This is very exciting for NHibernate but there's going to be a hiatus while the new implementation evolves.

Thanks to Tuna Toksöz for the info.

Vote For Me! (again)

Voting has opened for this year's Developer Developer Developer Day at Microsoft's UK Headquarters near Reading. You can vote for the sessions you want to see here:

http://www.developerday.co.uk/ddd/votesessions.asp

I've put forward a couple of talks:

Why do I need an Inversion of Control Container   (Style: Presentation - Level: 100)
Inversion of Control (IoC) containers such as Unity Windsor and Structure Map are a hot topic in the Microsoft Development world. What are they and why is there such a buzz about them? How will they help me build better applications? Join me on a trip to an parallel universe of application architecture where I show how IoC containers at last make component oriented software development a reality.

This will be an updated version of the talk I gave at last year's DDD. I'll try and compress the 'why' and have a little more of the 'what' this time.

Using an Inversion of Control Container in a real world application   (Style: Presentation - Level: 200)
Going beyond an initial introduction to IoC containers, this talk shows their use in an open source eCommerce application, Suteki Shop. I will show how the IoC container helps us write component oriented software and can significantly simplify both our architecture and code. This will include a look at some nice techniques such as generic repositories, using IoC containers with the MVC Framework and how they can help us host services.

A more in-depth look at how an IoC container helps with a real world application. I'm really looking forward to showing off the generic repository pattern and WCF integration in this one. The problem is going to be cramming everything in.

Taking the HttpContext out of the MVC Framework Controller

I really like the MVC Framework. I'm currently working on my third commercial project using it and I just love the flexibility it gives me, especially the opportunity to refactor the framework itself.

Here's an example: one of the things I dislike is the way the default Controller class is overly coupled to the HttpContext. HttpContext in itself is a problematic hangover from ASP.NET and to have it bound into the Controller to such an extent is a design mistake IMHO.

OK, so what do I mean? Take this action code from a controller:

public ActionResult Search()
{
    const int pageSize = 20;

    var criteria = new ArticleSearchCriteria();
    validatingBinder.UpdateFrom(criteria, Request.Form);

    var articles = articleRepository
        .GetAll()
        .ThatMatch(criteria)
        .ToPagedList(PageNumber, pageSize);

    return View("Search", View.Data
        .WithArticleSearchCriteria(criteria)
        .WithArticles(articles));
}

You see the reference to Request.Form there? The HttpRequest (and HttpResponse and HttpContext) are all exposed as properties of the base Controller class. In order to test that code I have to mock the HttpContext which is a real PITA.

Here's an alternative that I've started using. An IHttpContextService interface:

public interface IHttpContextService
{
    HttpContextBase Context { get; }
    HttpRequestBase Request { get; }
    HttpResponseBase Response { get; }
    NameValueCollection FormOrQuerystring { get; }
}

I can now just use Dependency injection in my controller to get a reference to an implementation of IHttpContextService. Note I don't care here how it is implemented:

public class ArticleController : ControllerBase
{
    private readonly IRepository<Article> articleRepository;
    private readonly IHttpContextService httpContextService;
    private readonly IValidatingBinder validatingBinder;

    public ArticleController(
        IRepository<Article> articleRepository, 
        IHttpContextService httpContextService,
        IValidatingBinder validatingBinder)
    {
        this.articleRepository = articleRepository;
        this.httpContextService = httpContextService;
        this.validatingBinder = validatingBinder;
    }

    public ActionResult Search()
    {
        const int pageSize = 20;

        var criteria = new ArticleSearchCriteria();
        validatingBinder.UpdateFrom(criteria, httpContextService.FormOrQuerystring);

        var articles = articleRepository
            .GetAll()
            .ThatMatch(criteria)
            .ToPagedList(PageNumber, pageSize);

        return View("Search", GroupLibView.Data
            .WithArticleSearchCriteria(criteria)
            .WithArticles(articles));
    }
}

And I can set up my IoC container to give me any implementation of IHttpContextService I want. Now because the IHttpContextService is provided by the IoC container, any dependencies that its implementation may require can also be provided by the IoC container which opens up all kinds of interesting opportunities.

Here's my current HttpContextService:

using System.Collections.Specialized;
using System.Web;

namespace Suteki.Common.Services
{
    public class HttpContextService : IHttpContextService
    {
        public HttpContextBase Context
        {
            get
            {
                return new HttpContextWrapper2(HttpContext.Current);
            }
        }

        public HttpRequestBase Request
        {
            get
            {
                return Context.Request;
            }
        }

        public HttpResponseBase Response
        {
            get
            {
                return Context.Response;
            }
        }

        public NameValueCollection FormOrQuerystring
        {
            get
            {
                if(Request.RequestType == "POST")
                {
                    return Request.Form;
                }
                return Request.QueryString;
            }
        }
    }
}

More on MSTest

I recently had a long and considered comment from Woonboo for my post "MSTest is sapping my will to live". Woonboo is a happy user of MSTest. I started writing a reply in the comments, but it was getting so involved I thought it would be nice to promote it to a new post.

Here's Woonboo's comment:

I know I'm late to the game, but mileage is everything.

I used to use NUnit, but after using MSTest for the past 2.5 years, I wouldn't go back, even with the bugs in VS2008 (which are minor and only hit if you are doing something specific with AppDomain).

Your config file thing is simple - you don't have to use attribute...in the 'settings' (right-click properties) add files to deploy.

It creates a separate test project because too many Morts out there will just put it in the same project otherwise and all your tests will be deployed with the code. I've worked with a number of these folks.

Having the built in ability to test private and internal methods/properties/fields is the biggest reason I love it. No extra code required. No need to loosen the scope (make things public) on what you're testing.

When a test fails, looking at the test results that gives me a hyper-link to every part of the stack trace where the test failed (or threw an exception) as saved me hundreds if not thousands of hours by now not having to navigate to the file, hit CNTL+G and enter the line number; especially if I have to walk up the stack to see if there was a path taken that shouldn't of been causing the problem.

Having the ability to have tests run automatically when I do a build or check-in (easier than I was ever able to with NUnit - but that's a SCC).

Code coverage gets tied to the tests you write.

Pulling up archives into the GUI of who ran what tests when on what machines. Great from the 'team lead' perspective.

You said in another post that technologies change but methodologies don't - don't let NUnit suck you into the 'one tool' mentality (although I could say the same about myself with MSTest). Love your posts generally - but I think you need to give MSTest another chance and ask someone who's been successful with it for help. It was hard for me to switch from NUnit too - but it was like when I switched from VB to C# - I never looked back once I got past the frustration point.

Woonboo's points get to the nub of the problem for me: I don't think MSTest was designed for TDD but for some other high-ceremony style of integration testing. I would submit that he is probably not working in that style. Let me take his points one by one to explain what I mean.

"you don't have to use attribute...in the 'settings' (right-click properties) add files to deploy". Yes, but I shouldn't have to do even this. Why can't it just test what's in the target directory? Building a separate run directory for every test and forcing the user to deliberately choose files to deploy is just another symptom of the high-ceremony MSTest approach.

"It creates a separate test project because too many Morts out there will just put it in the same project otherwise and all your tests will be deployed with the code." I agree that it's best to have a separate test project. But any testing framework should be unintrusive. I shouldn't have to have a separate project type. The only reason it's needed by MSTest is because MSTest requires so much configuration to be useful. And you shouldn't rely on project types to force you to correctly organise your source, otherwise we'd have some crazy Microsoft scheme to have the "domain-project", the "data-access" project, the "service-project". Don't suggest that too near to someone from the TFS team!

Now I have to have a rant here about "The Morts won't understand it" argument. I hear this over and over and over again. It usually goes along the lines of "I understand it fine, but the people I work with, or the 'maintenance' people won't". It's the lamest and most often deployed excuse for bad decisions. If the Morts can't understand something, whoever they are, get rid of them. What's the point in employing people who don't know how to do their job? But usually it's not the real reason, the real reason is lack of leadership, and lack of trust. Most advances in development practices actually make things simpler but it requires that you, if you are the team lead, sit down and work with the people you are supposed to be leading.

"Having the built in ability to test private and internal methods/properties/fields is the biggest reason I love it." One of the core reasons for doing TDD is way it drives your design. If you have to access private members in your tests I would submit that your design is not correctly decoupled. I *want* my testing framework to force me to behave like any other client when I'm writing tests.

Now there might be a situation where you have to write tests for some monolithic legacy code base, but MSTest won't help your there, you need to go and talk to Roy Osherove :)

"When a test fails, looking at the test results that gives me a hyper-link to every part of the stack trace where the test failed." You get exactly the same thing with Testdriven.NET. I have "run-tests" mapped to F8 (I've never found a use for bookmarks). "Run-tests" is context specific so if the cursor is currently inside a test method, only that test gets run. So I just hit F8 to run the test(s), the results appear on the console and I can click on any part of the stack trace to go directly to that line of code. I actually dislike fancy test runners. I don't want to have to click here and there to get to stack traces, I'd much rather everything was just thrown onto the console.

Of course running tests on your CI server is essential. I've always found it simple to do that both with Cruise Control and TFS. I don't think MSTest really adds much here and it doesn't make much sense unless you're using TFS.

"Pulling up archives into the GUI of who ran what tests when on what machines. Great from the 'team lead' perspective." OK, I don't get this. Surely what you should be looking at is code coverage. I run tests every few mintues when I'm coding, you probably wouldn't want to look at or store all those test runs. I think this goes back to my first point. A tool that's designed for high ceremony infrequent testing like MSTest would see storing a test run archive as a useful thing, anyone who's done TDD would see it as a huge waste of resources.

I gave MSTest a chance. I've also recently tried to use MBUnit. neither worked as well as TestDriven.NET + NUnit do for me. I must say that MBUnit came very close, and I wouldn't have too much problem with using if I was in a team that has already settled with it. MSTest was just a nightmare from start to finish. As I said to our project manager, if it was an open source tool, it would never have got any traction and would now be sitting unloved in sourceforge. Maybe it suits some people with a very non-agile, non-TDD methodology, but for anyone doing TDD I would stay well clear.

Woonboo. Thanks very much for provoking me to write this. I love a good debate and would really like to hear your reply. Thanks again!