Asp.Net MVC Controllers + BDD = The perfect match? Part 3: The AccountController contd.

Tags: , , , , , , , No Comments »

This is part 3 in a series of posts on using Behaviour Driven Development to build and test your MVC controllers. The full series is as follows:

In part one I defined a convention for how I was going to test my MVC controller functionality using JP Boodhoo’s developwithpassion library and in part two I took this a step further by building out some real specifications for the functionality that the out-the-box ASP.Net MVC AccountController provides. I dealt with the log on and log off functionality, which leaves us with the following requirements:

  • Register a new User
  • Change a User’s password
  • Prevent Window’s authenticated Users from accessing the application

Registering a new user

Registering a new user follows a similar pattern to the log on functionality. The user needs to be able to browse to the registration view and then submit their new user data to register a new account.

1. The first specification deals with being able to browse (HTTP GET request) to the register view. There is some added complexity in this specification, as the minimum password length value (used for validation purposes) is being retrieved from the membership service (remember I’m building these specifications backwards from the out-the-box functionality). In order to prove that this is happening as expected, I’m able to set up an expectation on my mock membership service that returns a value of 4, prove that the service was in fact called using the RhinoMocks VerifyAllExpectations() method and that the value is passed to the view by testing the value in the ViewData property bag. So far, so good.

My specification for displaying the register view is as follows:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_display_the_register_view : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        membership_service.Expect(ms => ms.MinPasswordLength).Return(4);

    because b = () =>
        result = sut.Register();

    it should_display_the_register_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_retreive_the_minimum_password_length_from_the_membership_service = () =>
        membership_service.VerifyAllExpectations();

    it should_display_the_minimum_password_length_in_the_view = () =>
        result.is_a_view_and().ViewData["PasswordLength"].should_be_equal_to(4);
}

2. As with the Log on specifications, I then test the “happy day” scenario for registering a new user. When everything goes to plan we should create the new user, log the user on to the system and redirect them back to the home page. Again, we can set up the mock membership service to return a successful registration status when we call it:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_register_a_user : concern_for_account_controller
{
    static ActionResult result;
    static MembershipCreateStatus success;

    context c = () =>
    {
        success = MembershipCreateStatus.Success;
        membership_service.Stub(ms => ms.CreateUser(user_name, password, email)).Return(success);
    };

    because b = () =>
        result = sut.Register(user_name, email, password, confirm_password);

    it should_create_the_user = () =>
        membership_service.was_told_to(ms => ms.CreateUser(user_name, password, email));

    it should_log_the_user_on_the_sytem = () =>
        forms_authentication.was_told_to(fa => fa.SignIn(user_name, false));

    it should_redirect_the_user_to_the_home_page = () =>
    {
        result.is_a_redirect_to_route_and().controller_name().should_be_equal_to("Home");
        result.is_a_redirect_to_route_and().action_name().should_be_equal_to("Index");
    };
}

3. Now we need to test the error cases. To register a new user, we need to specify a user name, email address, password and password again. We can simulate an invalid state for each scenario to prove that the controller handles the invalid input and responds accordingly. First, the user name:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_register_a_user_with_no_username : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        user_name = string.Empty;

    because b = () =>
        result = sut.Register(user_name, email, password, confirm_password);

    it should_display_the_register_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_display_the_username_validation_errors_in_the_view = () =>
        result.is_a_view_and().ViewData.ModelState["username"].should_not_be_null();
}

4. And then the same for email address:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_register_a_user_with_no_email : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        email = string.Empty;

    because b = () =>
        result = sut.Register(user_name, email, password, confirm_password);

    it should_display_the_register_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_display_the_email_validation_errors_in_the_view = () =>
        result.is_a_view_and().ViewData.ModelState["email"].should_not_be_null();
}

5. Then an invalid password:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_register_a_user_with_an_invalid_password : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        password = string.Empty;

    because b = () =>
        result = sut.Register(user_name, email, password, confirm_password);

    it should_display_the_register_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_display_the_password_validation_errors_in_the_view = () =>
        result.is_a_view_and().ViewData.ModelState["password"].should_not_be_null();
}

6. And then when the two passwords don’t match:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_register_a_user_with_password_that_dont_match : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        confirm_password = "different";

    because b = () =>
        result = sut.Register(user_name, email, password, confirm_password);

    it should_display_the_register_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_display_the_password_validation_errors_in_the_view = () =>
        result.is_a_view_and().ViewData.ModelState["_FORM"].should_not_be_null();
}

7. Finally, all the values may be ok, but the membership service may not let us register the user e.g. if the username already exists. In this case, we can simulate the membership service returning a failure response and check the desired behaviour:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_register_a_user_and_the_user_cant_be_created : concern_for_account_controller
{
    static ActionResult result;
    static MembershipCreateStatus failed;

    context c = () =>
    {
        failed = MembershipCreateStatus.ProviderError;
        membership_service.Stub(ms => ms.CreateUser(user_name, password, email)).Return(failed);
    };

    because b = () =>
        result = sut.Register(user_name, email, password, confirm_password);

    it should_try_to_create_the_user = () =>
        membership_service.was_told_to(ms => ms.CreateUser(user_name, password, email));

    it should_display_the_register_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_display_the_validation_errors_in_the_view = () =>
        result.is_a_view_and().ViewData.ModelState["_FORM"].should_not_be_null();
}

Change password

Changing a user’s password also follows a similar pattern – a user can browse to the change password view and then submit their new password details. My specifications follow the same pattern as before:

1. Check the correct view is returned when the user browses to the change password url

2. Test the happy day scenario when the user submits their data

3. Test the error case scenarios when the user submits invalid data

1. The specification for retrieving the change password view is similar to the one for the registration view:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_display_the_change_password_view : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        membership_service.Expect(ms => ms.MinPasswordLength).Return(4);

    because b = () =>
        result = sut.ChangePassword();

    it should_display_the_change_password_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_retreive_the_minimum_password_length_from_the_membership_service = () =>
        membership_service.VerifyAllExpectations();

    it should_display_the_minimum_password_length_in_the_view = () =>
        result.is_a_view_and().ViewData["PasswordLength"].should_be_equal_to(4);
}

2. The “happy day” scenario is slightly more complicated. In order to change the user’s password, the Account Controller needs to access the current user, which in the out-the-box functionality it does by just accessing the User property of the controller.

if (MembershipService.ChangePassword(User.Identity.Name, currentPassword, newPassword))
{
    return RedirectToAction("ChangePasswordSuccess");
}

This returns the IPrincipal associated with the current HTTP context, which means we need to simulate this in our specification. Faking an HTTP context is notoriously difficult to test, made slightly easier with the System.Web.Abstractions namespace. I’d prefer not to have any dependency on HttpContext in my controllers – which can be achieved by using ActionFilters – a technique that I describe in an earlier blog post.

However, the developwithpassion BDD framework helps us out on this one by providing an easy way to add further set up logic after we create the system under test. In this scenario we need to set up a mock HttpContext, assign a mock user and add it all to the AccountController’s ControllerContext.

Taking all this into account, our specification looks like this:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_change_a_users_password : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        membership_service.Stub(ms => ms.ChangePassword(user_name, password, new_password)).Return(true);

    after_the_sut_has_been_created set_up_the_current_user = () =>
    {
        sut.ControllerContext = new ControllerContext();
        sut.ControllerContext.HttpContext = an<HttpContextBase>();
        sut.ControllerContext.HttpContext.User = an<IPrincipal>();
        sut.ControllerContext.HttpContext.User.Stub(x => x.Identity).Return(an<IIdentity>());
        sut.ControllerContext.HttpContext.User.Identity.Stub(x => x.Name).Return(user_name);
    };

    because b = () =>
        result = sut.ChangePassword(password, new_password, confirm_new_password);

    it should_change_the_users_password = () =>
        membership_service.was_told_to(ms => ms.ChangePassword(user_name, password, new_password));

    it should_redirect_the_user_to_the_change_password_success_view = () =>
        result.is_a_redirect_to_route_and().action_name().should_be_equal_to("ChangePasswordSuccess");
}

3. The error case scenarios are then as before. Whenever I need to access the User property of the ControllerContext, I need to set it up in as above:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_change_a_users_password_with_no_current_password : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        password = string.Empty;

    because b = () =>
        result = sut.ChangePassword(password, new_password, confirm_new_password);

    it should_display_the_change_password_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_display_the_password_validation_errors_in_the_view = () =>
        result.is_a_view_and().ViewData.ModelState["currentPassword"].should_not_be_null();
}

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_change_a_users_password_with_an_invalid_new_password : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        new_password = string.Empty;

    because b = () =>
        result = sut.ChangePassword(password, new_password, confirm_new_password);

    it should_display_the_change_password_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_display_the_password_validation_errors_in_the_view = () =>
        result.is_a_view_and().ViewData.ModelState["newPassword"].should_not_be_null();
}

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_change_a_users_password_with_passwords_that_dont_match : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        confirm_new_password = "different";

    because b = () =>
        result = sut.ChangePassword(password, new_password, confirm_new_password);

    it should_display_the_change_password_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_display_the_validation_errors_in_the_view = () =>
        result.is_a_view_and().ViewData.ModelState["_FORM"].should_not_be_null();
}

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_change_a_users_password_and_the_password_cant_be_changed : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        membership_service.Stub(ms => ms.ChangePassword(user_name, password, new_password)).Return(false);

    after_the_sut_has_been_created set_up_the_current_user = () =>
    {
        sut.ControllerContext = new ControllerContext();
        sut.ControllerContext.HttpContext = an<HttpContextBase>();
        sut.ControllerContext.HttpContext.User = an<IPrincipal>();
        sut.ControllerContext.HttpContext.User.Stub(x => x.Identity).Return(an<IIdentity>());
        sut.ControllerContext.HttpContext.User.Identity.Stub(x => x.Name).Return(user_name);
    };

    because b = () =>
        result = sut.ChangePassword(password, new_password, confirm_new_password);

    it should_try_to_change_the_users_password = () =>
        membership_service.was_told_to(ms => ms.ChangePassword(user_name, password, new_password));

    it should_display_the_change_password_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_display_the_validation_errors_in_the_view = () =>
        result.is_a_view_and().ViewData.ModelState["_FORM"].should_not_be_null();
}

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_change_a_users_password_and_an_error_occurs : concern_for_account_controller
{
    static ActionResult result;

    context c = () =>
        membership_service.Stub(ms => ms.ChangePassword(user_name, password, new_password)).Throw(new Exception());

    after_the_sut_has_been_created set_up_the_current_user = () =>
    {
        sut.ControllerContext = new ControllerContext();
        sut.ControllerContext.HttpContext = an<HttpContextBase>();
        sut.ControllerContext.HttpContext.User = an<IPrincipal>();
        sut.ControllerContext.HttpContext.User.Stub(x => x.Identity).Return(an<IIdentity>());
        sut.ControllerContext.HttpContext.User.Identity.Stub(x => x.Name).Return(user_name);
    };

    because b = () =>
        result = sut.ChangePassword(password, new_password, confirm_new_password);

    it should_try_to_change_the_users_password = () =>
        membership_service.was_told_to(ms => ms.ChangePassword(user_name, password, new_password));

    it should_display_the_change_password_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();

    it should_display_the_validation_errors_in_the_view = () =>
        result.is_a_view_and().ViewData.ModelState["_FORM"].should_not_be_null();
}

4. Finally, when a password is successfully changed, the user is redirected to the “change password success” view. We have already tested that the redirection occurs in the “happy day” specification, so all that’s left is to prove that we can actually access that view:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_display_the_change_password_success_view : concern_for_account_controller
{
    static ActionResult result;

    because b = () =>
        result = sut.ChangePasswordSuccess();

    it should_display_the_change_password_success_view = () =>
        result.is_a_view_and().ViewName.should_be_empty();
}

Preventing Windows Authentication

The last thing which the out-the-box AccountController does for us, is deny access to users who are authenticated using Windows authentication. It does this by overriding the OnActionExecuting method (which will fire before any action method executes) and throws an exception if the current user is a WindowsIdentity:

protected override void OnActionExecuting(ActionExecutingContext filterContext)
{
    if (filterContext.HttpContext.User.Identity is WindowsIdentity)
    {
        throw new InvalidOperationException("Windows authentication is not supported.");
    }
}

Fortunately, this can easily be tested as each controller inherits the IActionFilter interface. As long as we cast our AccountController as an IActionFilter first, we can test this overridden method:

[Concern(typeof(AccountController))]
public class when_the_account_controller_is_told_to_do_something_and_the_current_user_is_authenticated_using_windows : concern_for_account_controller
{
    static ActionExecutingContext filter_context;
    static InvalidOperationException invalid_operation_exception;

    context c = () =>
    {
        filter_context = new ActionExecutingContext();
        filter_context.HttpContext = an<HttpContextBase>();
        filter_context.HttpContext.User = an<IPrincipal>();
        filter_context.HttpContext.User.Stub(x => x.Identity).Return(WindowsIdentity.GetAnonymous());
    };

    because b = () =>
    {
        try
        {
            ((IActionFilter)sut).OnActionExecuting(filter_context);
        }
        catch (InvalidOperationException ive)
        {
            invalid_operation_exception = ive;
        }
    };

    it should_throw_an_invalid_operation_exception = () =>
        invalid_operation_exception.should_not_be_null();
}

Summary

So I’ve now got a full set of executable specifications for the out-the-box AccountController. Although for these examples I’ve worked backwards from the functionality to the specifications, hopefully this provides an insight into how I would then go about building the functionality for new controllers in my application, or modifying/extending this existing functionality. The real benefit of BDD is that my specifications are easy to read and understand and describe the desired behaviour of the system, especially when running them through the specification parser, which gives me the following HTML output:

The process of writing these specifications for the AccountController has also highlighted that the AccountController is doing too much work around validation. In my opinion, the AccountController should pass off the validation responsibility to something else in order to keep the controller logic clean. Therefore, if the validation rules change, the AccountController behaviour does not need to. Behaviour Driven development helps with this process of deciding what the actual responsibility of a class really is. If you find you’re having to write too many specifications for a particular SUT, then maybe it’s doing too much. Single responsibility principle states that each class should have one responsibility – in the case of the controller this is governing the flow of the application. If the controller could delegate the validation to something else then it’s specifications and behaviour would be much simpler and your application less brittle.

In the final post I’ll provide the full source code for these examples and detail how then can be executed.

The importance of conventions – from Asp.Net MVC to a successful project team

Tags: , , , , , No Comments »

My colleague Jon George has recently been posting about his experiences of performance tuning our latest web application, built on Asp.Net MVC. His introductory post talks briefly about how we spent a large effort in laying the correct foundations for the project, which have ultimately led to its success. Being the “technical lead who had inconsiderately booked his wedding and an extended honeymoon right in the middle of the project”, I was obviously keen to ensure that that project didn’t fall down in a big heap whilst I was away. Although Jon would have you believe otherwise, my wedding and subsequent absence was actually planned well before we embarked on this project, so I knew from the offset that we had a risk around handover and consistency of delivery during project’s lifetime. To add to this, this was my first “official” Dev Lead role for EMC Consulting so I wanted to make sure that we got things right from the offset.

Howard and I started planning and brainstorming how we were going to approach this project well before sprint zero. We were lucky to have few technical restrictions, but the side effect of this was that we had a vast amount of decisions to make, with a vast number of options for each choice. We decided right from the offset that we were going to use Asp.Net MVC. We both have a background in web development, before .Net and Webforms, so moving back to a more “webby” web framework was something that we were really keen to do. We’d also hit a lot of pain-points in previous projects with Webforms and we were hoping MVC would allow us to get round these and do things the way we wanted to. MVC Version 1.0 had literally just been released, so we were good to go.

One of the main benefits mentioned when discussing Asp.Net MVC is the use of convention over configuration – a design principle that aims to reduce the number of decisions that developers need to make. The result of this is consistency, less code and an overall decrease in development time.

For example, in the MVC framework all controller classes are named with the suffix “controller”. So, if you have a controller responsible for search functionality, you know you need a SearchController.

Removing a decision needed to be made by a developer (in this case, what to call a controller class), albeit on a minute scale, adds up when when you apply it time and time again over lots of different conventions throughout the lifetime of a project. Asp.Net MVC does not enforce these conventions by any means, and practically everything is overridable, customisable or configurable – but each time you break from a convention, you’re adding time, complexity and inconsistency to your project solution. In short, if you follow the conventions, things are quick, easy and consistent. Happy days.

A lot of the communities around Asp.Net MVC have adopted this love of conventions too. We built our site around Billy McCafferty’s excellent Sharp Architecture, which is a “architectural reference for a best practice Asp.Net MVC web application” i.e. it defined the conventions on what we needed in an MVC solution, were everything would live and how it would all talk to each other. This, in turn is built upon James Gregory’s Fluent NHibernate – a project solely concerned with defining a convention based approach to using NHibernate, in order to eliminate the need for manually editing XML mapping files. Like I say, conventions at a minute scale all add up to save time, remove decisions and increase consistency within a solution.

The suprising thing was however, over the course of the project, this convention based approach extended out of the MVC solution and started to permeate other parts of the project and the team. We’d set a firm foundation of how we were going to build the code, but we had also been setting firm foundations of how the project was going to run.

As Jon mentioned, I’m a big fan of Getting Real and set out from the beginning with it’s principles in mind. We had a fixed budget and a fixed delivery date, so the scope of what we were delivering had to be the thing that moved. We were really cutthroat in determining what was “core” functionality for our site to launch. This was drilled into everyone who got involved with the team over the course of the project in order to eliminate scope creep from all angles. “We are only building core functionality” became my mantra and resulted in a convention that everyone understood, which was to say no to any new feature requests. If the client pushed, then they were added to the end of the backlog, well away from the core items that were to be developed first.

We were told to “spend the money as if it were our own”. We adopted this mantra right from sprint zero, evaluating and spiking a multitude of open source technology which we baked into our solution from the beginning. This convention of challenging every cost enabled us to validate and prioritise our requirements. “What’s the added business value?” is a question that would be asked a lot if the team thought we were being asked to develop something they thought was giving little benefit. Being able to challenge functionality allowed us to really concentrate on our core features – which ultimately lead to us delivering the project ahead of schedule.

I set a rule in the first sprint that every team member would demo to the client at the end of every sprint. Whilst sprint demos are not a new thing to most people here, not everyone always gets the chance to demo their work, for various reasons – team size, time constraints, client involvement etc. I wanted to instil a convention right from the beginning that everyone in the team would demo what they had done during the sprint. So, we had designers demoing new creative work, developers demoing functionality, testers talking about bug counts, cross browser issues and running automated UI tests, database developers demoing BI and ETL processes. Whilst not all of this interests everybody, there was never any question of whether someone would be required to “show and tell” what they’d been up to. The result of this was that we never wasted time during a sprint discussing who would demo what and everyone made damn sure that what they were doing worked by the end of the sprint.

The combined effect of all these conventions is that everyone on our core project team is able to explain “this is how we do things on this project and these are the reasons why”. That’s a powerful message. One of the main benefits of working in a consultancy is that we have experts in a long list of specialities and whilst those skills are not needed full time for a project, its great to be able to draw on that experience as and when required. We had a core team of less than 15 people, yet over the lifetime of the project, over 30 of our consultants had provided direct input into the end result. Without embedding, understanding and enforcing our project’s conventions within the core team, this could have been messy business.

So, what’s the point in all this? Well, as Jon stated, I “abandoned” the project right in the middle to take a 6 week honeymoon. (pause for memories of sun, sand and… well, you get the idea). I was confident when I left that the project was in safe hands – we already had a great team and Jon is a great Dev Lead himself. But I don’t think I realised just how well everything was going to go. We had a brief handover where we discussed loosely how I thought features were going to pan out and left it at that. I returned after 6 weeks to find that

  • The project kept momentum and had considerably moved forward in terms of functionality
  • The features we had discussed had been developed in the way that we talked about – with no issues
  • Jon had quickly engaged and developed a relationship with the client as he understood the solution and how we were running the project
  • Everyone had been demoing their feature-complete work at the end of every sprint
  • No new features had been added to the backlog
  • I was able to easily pick up the solution where I left off and continue to work on the new features that had been developed
  • The client was still happy!

The project didn’t fall down in a big heap whilst I was away, in fact it carried on at the same pace, quality and direction that it had before I left. That could actually say something about me! Or probably more about Jon! But I think most of all it says that if you spend time getting your conventions right then a lot of things will just fall into place. And that makes for a smooth running project.

Design by j david macor.com.Original WP Theme & Icons by N.Design Studio