Mimicking Html.BeginForm() to reduce html div duplication in Asp.Net MVC sites

Recently I have been trying to find ways to reduce how much HTML I have to duplicate in my views and how I have to remember what css classes to give each set of divs. The problem comes with that the HTML that my views require don’t fit in the main layout, due to the fact that content still comes after it and some elements are optional, and they don’t fit well in partial views due to how customized the HTML inside the area is.

An example of the HTML I was dealing with, here’s an example of one of my views

<div class="grid1 floatLeft"> 
    <div class="lineSeperater"> 
        <div class="pageInfoBox"> 
            @using (Html.BeginForm(MVC.JobSearch.Edit())) 
            {
                @Html.HiddenFor(x => x.Id)
                
                <div class="grid3 marginBottom_10 marginAuto floatLeft"> 
                    <h3 class="floatLeft">@(isNewJobSearch ? "Start Job Search" : "Edit Job Search")</h3> 
                </div> 
                
                <div class="grid3 marginBottom_10 marginAuto floatLeft">
                    <div class="floatLeft">
                        <p>
                            Displayed page summary
                        </p>    
                    </div>
                </div>
                
                <div class="grid3 marginBottom_10 marginAuto floatleft">
                    <div class="floatLeft infoSpan">
                        @Html.ValidationSummary()
                    </div>
                </div>
                
                <div class="grid3 marginBottom_10 floatLeft"> 
                    <div class="floatLeft"><p class="greyHighlight">Title:</p>
                        <div class="infoSpan">@Html.TextBoxFor(x => x.Name, new { @class = "info" })</div>
                    </div> 
                </div> 

                <div class="grid3 marginBottom_10 floatLeft"> 
                    <div class="floatLeft"><p class="greyHighlight">Description:</p>
                        <div class="infoSpan">@Html.TextAreaFor(x => x.Description, new { @class = "textAreaInfo" })</div>
                    </div> 
                </div> 

                <div class="grid3 marginBottom_20 floatLeft"> 
                    <div class="submitBTN "><input type="submit" value="Save" /></div>                    
                </div> 
            }

            <div class="clear"></div> 
        </div> 
    </div> 
</div> 

<!-- More HTML Here -->

I started thinking of how I can increase my code re-use to make this easier to develop and maintain. While looking over the view my eyes gravitated towards the Html.BeginForm() and I realized the most logical idea was to utilize using statements. So after looking at the implementation of Html.BeginForm() for guidance (thanks to dotPeek) I came up with the following class to implement the first few divs automatically.

    public class PageInfoBoxWriter : IDisposable
    {
        protected ViewContext _viewContext;
        protected bool _includesSeparator;

        public PageInfoBoxWriter(ViewContext context, bool includeSeparator)
        {
            if (context == null)
                throw new ArgumentNullException("context");

            _viewContext = context;
            _includesSeparator = includeSeparator;

            // Write the html
            _viewContext.Writer.Write("<div class=\"grid1 floatLeft\">");

            if (_includesSeparator) _viewContext.Writer.Write("<div class=\"lineSeperater\">");

            _viewContext.Writer.Write("<div class=\"pageInfoBox\">");

            return;
        }

        public void Dispose()
        {
            _viewContext.Writer.Write("<div class=\"clear\"></div></div></div>");
            if (_includesSeparator) _viewContext.Writer.Write("</div>");
        }
    }

I then created an html helper to use this class

    public static class LayoutHelpers
    {
        public static PageInfoBoxWriter PageInfoBox(this HtmlHelper html, bool includeSeparator)
        {
            return new PageInfoBoxWriter(html.ViewContext, includeSeparator);
        }
    }

This will write the beginning divs upon creation and the div end tags upon closing. After following suit with the other elements of my page and creating more disposable layout classes and Html helpers, I now have the following view:

@using (Html.PageInfoBox(false))
{
    using (Html.BeginForm(MVC.JobSearch.Edit())) 
    {
        @Html.HiddenFor(x => x.Id)

        using (Html.OuterRow())
        {
            <h3 class="floatLeft">@(isNewJobSearch ? "Start Job Search" : "Edit Job Search")</h3> 
        }

        using (Html.OuterRow())
        {
            <div class="floatLeft">
                <p>
                    Displayed page summary
                </p>    
            </div>
        }

        using (Html.OuterRow())
        {
            <div class="floatLeft infoSpan">
                @Html.ValidationSummary()
            </div>
        }

        using (Html.OuterRow())
        {
            Html.FormField("Title:", Html.TextBoxFor(x => x.Name, new { @class = "info" }));
        }

        using (Html.OuterRow())
        {
            Html.FormField("Description:", @Html.TextAreaFor(x => x.Description, new { @class = "textAreaInfo" }));
        }

        using (Html.OuterRow())
        {
            using(Html.FormButtonArea())
            {
                <input type="submit" value="Save" />
            }
        }
    }
}

<!-- other html here -->

Now I have a MUCH more maintainable view that even gives me intellisense support, so I don’t have to worry about remembering how css classes are capitalized, how they are named, what order the need to be in, etc…

Asp.Net Membership Provider’s Lifetime Considerations- Part 2

Previously I made a post about issues I encountered with the Asp.Net Membership Provider.

My fix was a bit short-sighted as it only fixed the issue in one location, when the account controller tries to perform operations. The problem with my solution is that it does not deal with all the areas where Asp.Net creates the membership provider itself, and keeps that instance running through the application lifetime.

This meant that all calls to the membership provider (e.g. calling Membership.GetUser()) were all using the same database session, even across multiple web requests. Not only did this cause hidden caching issues, this also mean that if the database connection was broken all calls to the membership provider would now exception out with the only way to fix it is to restart the Asp.Net instance.

In order to fix this I had to remove my database session creation out of my custom membership provider’s constructor, and instead retrieve a new database session from my IoC system inside each method.

This seems to not only fix my original issue, but several other “random” exceptions that I could not reproduce on a regular basis.

Beware of Asp.Net’s Membership Provider Lifetime, Entity Framework Caching, and Dependency Injection

I recently struggled while dealing with a bug I encountered in my Asp.Net MVC application, and I thought I would write about it to hopefully help someone else.

I have a method in my business layer that users use to change their password. After implementing unit tests to verify the method worked properly I implemented a call to it in my account controller. Everything seemed perfect at first, as I was able to change my password successfully and even validate that the user entered their correct current password, which is required to perform a password change.

However, when I logged out and tried to log back in using the new password the login attempt failed. Yet when I used my previous password I was able to log in! To make things even more confusing, since I require users to enter their current password to change their password I was able to confirm that the password change did actually take effect. Finally, when I made a debugging change and re-ran the app, the new password worked when logging in!

A few irritating hours later, I finally figured out what was wrong. It came down to the difference of lifetimes in my MVC application between different classes. My AccountMembershipService class, which was mostly based on the default code that came with MVC, had the following constructor:

        public AccountMembershipService()
            : this(null)
        {
        }

        public AccountMembershipService(MembershipProvider provider)
        {
            _provider = provider ?? Membership.Provider;
        }

The problem is that my application loads its service classes using Castle Windsor, and entity framework is loaded from Windsor with a per-web request lifetime. Even though my custom membership provider was retrieving the database context from Windsor, Asp.Net creates the membership provider for the lifetime of the whole application.

So what happened was that I would log in using the database context for the Membership Provider, but when I changed my password the service class would use a different database context. When I changed my password, the password is correctly changed in the database (and the request’s specific database context), but the Membership Provider is still using the original database context. Since Entity Framework caches previously received entities, when I go back to log in entity framework receives the user entity out of the cache, not the database, and thus the old password is what will pass the Membership Provider’s validation check.

The fix was to replace the previous code with:

        public AccountMembershipService(MembershipProvider provider)
        {
            _provider = provider ?? new MyMembershipProvider();
        }

This forces a new membership provider instance to be created for that web request, and thus guarantees that the database context will not be holding a stale user entity.

Hopefully this saves someone from the irritation and wasted time that I went through.


Update: I have written another post adding on to the issues I wrote about here.

Keeping Asp.NET MVC Controller Constructors Clean In a Dependency Injection World – Part 2

Previously I wrote a post an article about a way to keep Asp.net MVC constructors clean with heavy dependency injection. In that post, which can be found here, proposed creating the following class to resolve dependencies on demand:

public class WindsorServiceFactory : IServiceFactory
{
    protected IWindsorContainer _container;

    public WindsorServiceFactory(IWindsorContainer windsorContainer)
    {
        _container = windsorContainer;
    }

    public ServiceType GetService<ServiceType>() where ServiceType : class
    {
        // Use windsor to resolve the service class.  If the dependency can't be resolved throw an exception
        try { return _container.Resolve<ServiceType>(); }
        catch (ComponentNotFoundException) { throw new ServiceNotFoundException(typeof(ServiceType)); }
    }
}

This seemed like a good idea in theory, even though I got told by people on here and on Ayende’s blog that it was a bad idea. After using this in a project, I can now agree that this is a bad idea.

Dependency Ambiguity

One problem is that it doesn’t make it clear what dependencies a class has, and those ambiguities are hidden behind the implementation details. At first this doesn’t seem like a big deal, but it can cause confusion down the road. An example of this is when you have a method that retrieves data about a post in your blog system. When you write a unit test this might fail on a private blog because it makes a call to another class to do a security check (make sure the user has access to the blog or post). However, there is no way to know what service class is going to be used for this call without knowing the implementation details of how the IServiceFactory.GetService() call is made. Instead, when this service class interface is passed in via IoC into the constructor, it is clear to any outside users what service interface is required to use.

IoC Testing

Another (major in my opinion) issue is you cannot easily test that ALL dependencies can be resolved. In order to unit test that all of my dependencies can be resolved by IoC, I have to explicitly create a unit test for each type of class I *could* possibly use in my business layer or presentation layer. I say could because since these dependencies are not being passed in through constructors I don’t know exactly what IServiceFactory.GetService calls will be made. The only way to be 100% sure I added everything to my IoC is by manually running through every path of my application. This is error pron and just bad.

Instead, when you pass dependencies into your constructors, testing that ALL dependencies can be resolved is simple. All you have to do is create one unit test per MVC constructor that looks like:


[TestMethod]
public void Windsor_Can_Resolve_HomeController_Dependencies()
{
	// Setup
	WindsorContainer container = new WindsorContainer();
	container.Kernel.ComponentModelBuilder.AddContributor(new SingletonLifestyleEqualizer());
	container.Install(FromAssembly.Containing<HomeController>());

	// Act
	container.Kernel.Resolve(typeof(HomeController));
}

This will not only verify that all my HomeController dependencies can be resolved, but that all child and grandchild dependencies can be resolved as well. This is MUCH less error prone.

So What Now

So now we come to the question that brought me here in the first place, how do I control constructor bloat when using dependency injection? After thinking about it, it really feels like I have been trying to squash an ant by using a bulldozer. The REAL problem with constructor bloat is that your MVC controllers are doing too much. Once you begin to realize this, the stupidity of my ServiceFactory stuff comes clear. Constructors doing too much have more issues than just constructor bloat, it also makes it hard to navigate through controller actions.

So the real solution to the problem? Utilize areas and spread out your constructor responsibilities among multiple MVC controllers instead of trying to fit all the actions into one controller, even if they seem vaugely related. This will keep your MVC application much more maintainable in the long run, and easier to debug.

Keeping Asp.NET MVC Controller Constructors Clean In a Dependency Injection World

Update: After playing around with the concepts I talked about in this post I now pretty much disagree with what I wrote here. To see why, see part 2.


In my experience, dependency injection has proven to be invaluable when creating applications. It has allowed me to easily follow test driven development practices, be confident that my application functions as expected, and it gives me confidence that any bugs I had previously found do not crop up again.

However, one trend that I have noticed is that as my Asp.NET MVC application becomes more complex, my controller constructors are becoming bloated with all of the service objects that the controller might use. I say might because not all the service objects are being used in all actions, but they must be passed into the controller’s constructor to facilitate unit testing. This issue of constructor bloat is made worse due to my business/service layer being made up of many small query and command classes. Here is an example of one of my controllers:


public class ContactController : MyAppBaseController
{
    public ContactController( ContactsByCompanyQuery contactsByCompanyQuery,
                                        ContactByIdQuery contactByIdQuery,
                                        CreateContactCommand createContactCommand,
                                        EditContactCommand editContactCommand,
                                        ISearchProvider searchProvider,
                                        IEmailProvider emailProvider)
    { 
        // copy parameters into variables
    }
}

As you can see from this example, as I add more actions to my controller and need more service classes I have to add more parameters into my controller, which also means I have to update all unit tests to use this new parameter when instantiating the controller. This also strikes me as inefficient as it requires the inversion of control system to always instantiate all service classes even when the executing MVC action may only need one or two.

I needed a system that would allow me to minimize the number of parameters in my controller constructors while still giving me full flexibility with dependency injection and retaining the ability to unit test my controllers and mock my service classes. After thinking about this problem for a bit I came up with the idea of creating a factory class that instantiates service classes on demand directly from the IoC system. As I use Castle Windsor for IoC I created the following factory class:

    public class WindsorServiceFactory : IServiceFactory
    {
        protected IWindsorContainer _container;

        public WindsorServiceFactory(IWindsorContainer windsorContainer)
        {
            _container = windsorContainer;
        }

        public ServiceType GetService<ServiceType>() where ServiceType : class
        {
            // Use windsor to resolve the service class.  If the dependency can't be resolved throw an exception
            try { return _container.Resolve<ServiceType>(); }
            catch (ComponentNotFoundException) { throw new ServiceNotFoundException(typeof(ServiceType)); }
        }
    }

It turned out to be a much simpler solution than I thought it would be. Now my constructors look like

public class ContactController : MyAppBaseController
{
    protected IServiceFactory _serviceFactory;

    public ContactController (IServiceFactory factory)
    {
        _serviceFactory = factory;
    }

    public ActionResult Search(string query)
    {
        var results = _serviceFactory.GetService<ISearchProvider>().Search(query);
        return View(results);
    }
}

This is much cleaner in my opinion and it also allows me to completely ignore my constructors even as I add more functionality into my controllers.

I am also able to easily use this in my unit tests with mocks. Here is an example using Moq:

[TestMethod]
public void Can_Search_Contacts()
{
    // Setup
    var factory = new Mock<IServiceFactory>();
    var searchProvider = new Mock<ISearchProvider>();
    factory.Setup( x => x.GetService<ISearchProvider>()).Returns(searchProvider.Object);
    var controller = new ContactController(factory.Object);
    
    // Act
    var result = controller.Search();

    // Add verifications here
}

Disappointed with Asp.net MVC’s testability

Update:Since writing this post I found a useful tool called the Razor Generator. Following this blog post I am now able to effectively unit test Asp.NET MVC views. At this point I am happy, although I don’t know if this is a Razor only solution.


I have been happily using the Asp.NET MVC framework for personal and professional projects for about a year now.  I love how easy and straight-forward it has made web development for .NET developers, and using it has been nothing but pure joy.  However, I have noticed one nagging issue that I keep coming up against, and that issue is its advertised testability.

Background

First some background.  I have a personal website that was brought into a friends and family beta last month (unfortunately, it is not ready to be revealed to the public yet).  I was feeling good about bringing real users to the site, as it’s a relatively small scale site with 198 implemented unit tests that currently test all aspects of the backend. I also manually tested everything I could think of in the front end and everything seemed to be working great in its current state.  As a QA professional, I knew there would certainly be bugs that I missed, but I thought that they would be the less obvious bugs that came about due to real-world usage of the system.

Unfortunately, right when we brought our first user to the site I immediately got an email of a failure with the system. I followed the user’s instructions, which turned out to consist of only one step, to visit the registration page. Upon loading the registration page I immediately saw a crash in the MVC view.  It turns out the issue was that I had moved my MVC view models from the `MyApp.Models` namespace to the `MyApp.ViewModels.Accounts` namespace for better organization.

The annoying aspect of this error was not the error itself, I merely missed the registration page when I reorganized my view models.  The annoying part was that none of my existing unit tests failed due to this.  Even if you use MVCContrib’s AssertViewRendered() method, a compile or run-time error in a view will not cause the test to fail.

It turns out that neither MVCContrib’s AssertViewRendered() nor any other example actually executes an action’s view to make sure it actually renders. All it does is make sure it returns a ViewResult data type.

Lack of Testability

As far as I can tell, there is no way to test for errors in an Asp.NET MVC view. Some people are probably saying in their heads right now that I could just enable compile error checking via the MVCBuildViews property in the project’s configuration management. There are two problems with this idea.

The first issue is that compiling views on every recompile causes the compile process to be ridiculously slow, and it’s impracticable to do this all the time during development. It’s possible to have a special configuration setup just for precompiling the views, but it requires the developer to remember to test-compile the project under that configuration prior to deployment. From my experiences in QA, requiring a developer to remember to do something like this is asking for trouble, and even I have forgotten to recompile my code under the special configuration prior to deployment.

The second issue with relying on MVCBuildViews is that it does not help find runtime errors that are in views. For example, you can test that your controller is sending a view model of type MyApp.ViewModels.Accounts.RegistrationViewModel, but it does nothing to make sure the view is actually expecting a view model of that type, and if the view is coded to use a different view model type then there is no way during development to know that the two are not in sync. The only way to know if the controller and view are out of sync is to manually navigate to the page in a web browser. More of these non-testable errors that can occur in a view are casting errors, bad loops, bad data in the view model, null data in the view model, etc….

In order to test for an error in a view, you have to take the returned ViewResult data structure and run the ExecuteResult() method. Unfortunately, I cannot get the ExecuteResult() method tested in an integration test as errors occur. For example, I have created the following test:

        [TestMethod]
        public void RegisterResultExecutes()
        {
            //arrange 
            RouteData routeData = new RouteData();
            routeData.Values.Add("action", "register");
            routeData.Values.Add("controller", "Account");

            RequestContext requestContext = new RequestContext(new MockHttpContext(), routeData);
            AccountController controller = new AccountController
            {
                FormsService = new MockFormsAuthenticationService(),
                MembershipService = new MockMembershipService(),
                Url = new UrlHelper(requestContext)
            };

            ViewResult result = controller.Register() as ViewResult;
            var sb = new StringBuilder();
            Mock<HttpResponseBase> response = new Mock<HttpResponseBase>();
            response.Setup(x => x.Write(It.IsAny<string>())).Callback<string>(y =>
            {
                sb.Append(y);
            });

            Mock<ControllerContext> controllerContext = new Mock<ControllerContext>();
            controllerContext.Setup(x => x.HttpContext.Response).Returns(response.Object);
            controllerContext.Setup(x => x.RouteData).Returns(routeData);

            //act 
            result.ExecuteResult(controllerContext.Object);
        } 

This unit test fails with a NullReferenceException with the following stack trace:

System.Web.Compilation.BuildManager.GetCacheKeyFromVirtualPath(VirtualPath virtualPath, Boolean&amp; keyFromVPP)
System.Web.Compilation.BuildManager.GetVPathBuildResultFromCacheInternal(VirtualPath virtualPath, Boolean ensureIsUpToDate)
System.Web.Compilation.BuildManager.GetVPathBuildResultInternal(VirtualPath virtualPath, Boolean noBuild, Boolean allowCrossApp, Boolean allowBuildInPrecompile, Boolean throwIfNotFound, Boolean ensureIsUpToDate)
System.Web.Compilation.BuildManager.GetVPathBuildResultWithNoAssert(HttpContext context, VirtualPath virtualPath, Boolean noBuild, Boolean allowCrossApp, Boolean allowBuildInPrecompile, Boolean throwIfNotFound, Boolean ensureIsUpToDate)
System.Web.Compilation.BuildManager.GetVirtualPathObjectFactory(VirtualPath virtualPath, HttpContext context, Boolean allowCrossApp, Boolean throwIfNotFound)
System.Web.Compilation.BuildManager.GetObjectFactory(String virtualPath, Boolean throwIfNotFound)
System.Web.Mvc.BuildManagerWrapper.System.Web.Mvc.IBuildManager.FileExists(String virtualPath)
System.Web.Mvc.BuildManagerViewEngine.FileExists(ControllerContext controllerContext, String virtualPath)
System.Web.Mvc.VirtualPathProviderViewEngine.GetPathFromGeneralName(ControllerContext controllerContext, List`1 locations, String name, String controllerName, String areaName, String cacheKey, String[]&amp; searchedLocations)
System.Web.Mvc.VirtualPathProviderViewEngine.GetPath(ControllerContext controllerContext, String[] locations, String[] areaLocations, String locationsPropertyName, String name, String controllerName, String cacheKeyPrefix, Boolean useCache, String[]&amp; searchedLocations)
System.Web.Mvc.VirtualPathProviderViewEngine.FindView(ControllerContext controllerContext, String viewName, String masterName, Boolean useCache)
System.Web.Mvc.ViewEngineCollection.&lt;&gt;c__DisplayClassc.b__b(IViewEngine e)
System.Web.Mvc.ViewEngineCollection.Find(Func`2 lookup, Boolean trackSearchedPaths)
System.Web.Mvc.ViewEngineCollection.FindView(ControllerContext controllerContext, String viewName, String masterName)
System.Web.Mvc.ViewResult.FindView(ControllerContext context)
System.Web.Mvc.ViewResultBase.ExecuteResult(ControllerContext context)
MyApp.Tests.Controllers.AccountControllerTest.RegisterResultExecutes() in C:\Users\KallDrexx\Documents\Projects\MyApp\MyApp.Tests\Controllers\AccountControllerTests.cs: line 303

I can’t tell if it’s possible to get any further from here, and I am beginning to think that creating an integration test on views is impossible. This makes me a bit sad and confused, sad because Asp.NET MVC heavily advertises the fact that it’s extremely testable and confused because I can’t seem to find any discussion on this at all, and I can’t believe that I am the only one interested in having automated tests that verify exceptions do not occur when rendering a view. If this isn’t testable then I am not quite sure how MVC is any more testable than a well architected webforms application.

As of now I will probably have to either have unit tests that use HttpWebRequests (if it’s possible due to parts of the site requiring forms authentication) or tests that use the Watin framework, neither of which are particularly appealing and add additional points of possible failure.