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.

A ViewModel Based Data Access Layer – Persisting Data

So far in my design of a Data Access Layer that I outlined in my last two posts dealt with retrieving data from the database, so now I want to explore the second half of a data access layer, how you persist data in the database.

Facade Of Simplicity

Contrary to what the repository pattern would have you believe, saving data in a data store isn’t always a simple process that is the same in all situations. Sometimes persisting data in the database requires special optimizations that are only valid in specific scenarios. To illustrate this, let’s again look at a process of saving a question at Stack Overflow with a traditional ORM.

When a user wants to add a question the process is simple, we just create a new instance of the question POCO and tell our ORM to save it to the database. We want to save all of it’s data because it’s all new. However what about when the use wants to edit his question? In all ORMs that I know of you must run a query to retrieve the record from the database, populate your POCO with the data, save the changes the user made to the question, then send the modified object back to the database for saving.

This has several inefficiencies to it. For starters it requires you to completely retrieve the data for the question from the database when none of it is actually relevant to us. To save the edits for a question we don’t care about how many upvotes it has, how many downvotes, the original date of posting, the user who made the original post of the question, we don’t even care what the original question was. All we care about is the new title, text, and tags for the question and we want those persisted, so why retrieve all that data. This may seem insignificant but when your application gets a lot of traffic this can cause a lot of repeated and unneeded traffic between your database and webserver. Since the database is usually the bottleneck in a web application, and there are situations you can be in with Sql Azure where you pay for DB bandwidth, this can end up costing you in the long run. Also consider the effect of having to mass update multiple records.

Most ORMs (or any one worth it’s salt) have some methods around this. The first usually involves creating a new POCO object with the modified data and telling the ORM that it should save the object as is. This is usually bad because it requires the POCO to already know all the data it probably shouldn’t, such as the number of upvotes, date it was created, etc.. If any of these properties aren’t set, then using this method will cause them to be nulled or zeroed out and will most likely cause data issues. It is very risky to do this, at least with Entity Framework. Another way around the inefficiency is to tell the ORM to use straight SQL to update the record, thus bypassing the the safety-net and security of ORM.

Both of these methods have their individual situations where they are beneficial, but rarely do you want to use one or the other all the time. Trying to abstract each of these situations into a data access layer that is database agnostic isn’t simple.

Persisting Data

So now the question becomes, how can I persist data in my ViewModel based DAL. To do this I have come up with the following interface:

public interface INonQuery<TViewModel, TReturn>
{
	TReturn Execute(TViewModel viewMode);
}

This interface states that you want to take the data from a specific view model and save it to the database, with a requested return type. This allows the developer to do performance optimizations on a situation by situation basis, but if need-be they can keep the implementation consolidated until they feel they need that optimization without breaking the application layer using the API

A ViewModel Based Data Access Layer – Optionally Consolidated Query Classes

After thinking upon my data access pattern outlined in my last post, I came up with the following interface to use for queries:

public interface IQuery<TViewModel, TCriteria>
{
	TViewModel Execute(TCriteria criteria);
}

I am very happy with this idea, as it seems to have multiple advantages over current data access patterns I have evaluated.

Optionally Consolidated Queries

One advantage that I see of this pattern is that it’s trivial to combine similar queries into one class, while keeping the business layer ignorant of the grouping.

To show this let’s use the example of Stack Overflow. If you look at the homepage and your user’s page you will notice that in these pages there are 2 queries that are retrieving data for a specific user, but each query returns different information. The homepage only requires the user’s username, reputation, and badge data. However, when you view a user’s page it needs that to query for that information as well as questions and answers related to the user. Even though both queries deal with retrieving data for a specific user it would be inefficient to use the latter query for the homepage, as it would have to hit multiple tables when that data isn’t used.

An example of creating an MVC controller that uses my ViewModel DAL would be:

public class ExampleController : Controller
{
	protected IQuery<UserHomepageViewModel, UserByIdCriteria> _userHomepageQuery;
	protected IQuery<UserDashboardViewModel, UserByIdCriteria> _userDashboardQuery;
	protected int _currentUserId;
	
	public ExampleController(
		IQuery<UserHomepageViewModel, UserByIdCriteria> userHomepageQuery,
		IQuery<UserDashboardViewModel, UserByIdCriteria> userDashboardQuery)
	{
		_userHomepageQuery = userHomepageQuery;
		_userDashboardQuery = userDashboardQuery;
		_currentUserId = (int)Membership.GetUser().UserProviderKey;
	}
	
	public ActionResult Index()
	{
		var criteria = new UserByIdCriteria { UserId = _currentUserId };
		var model = _userHomepageQuery.Execute(criteria);
		return View(model);
	}
	
	public ActionResult UserDashboard()
	{
		var criteria = new UserByIdCriteria { UserId = _currentUserId };
		var model = _userDashboardQuery.Execute(criteria);
		return View(model);
	}	
}

As far as the programmer in charge of this controller is concerned, two completely separate classes are used for these queries. However, the developer can save some effort by implementing these queries into one consolidated class. For example:

public class UserByIdQueries : IQuery<UserHomepageViewModel, UserByIdCriteria>, IQuery<UserDashboardViewModel, UserByIdCriteria>
{
	protected DbContext _context;
	
	public UserByIdQueries(DbContext context)
	{
		_context = context;
	}
	
	public UserHomepageViewModel Execute(UserByIdCriteria criteria)
	{
		var user = GetUserById(criteria.UserId, false);
		return Mapper.Map<User, UserHomepageViewModel>(user);
	}
	
	public UserDashboardViewModel Execute(UserByIdCriteria criteria)
	{
		var user = GetUserById(criteria.UserId, true);
		return Mapper.Map<User, UserDashboardViewModel>(user);
	}
	
	protected User GetUserById(int id, bool includeDashboardData)
	{
		var query = _context.Users.Where(x => x.Id == id);
		
		if (includeDashboardData)
			query.Include(x => x.Questions).Include(x => x.Answers);
			
		return query.SingleOrDefault();
	}
}

To me, this gives the perfect balance of easily retrieving data from the DAL based on how I am going to use the data and still give me full flexibility on how I organize and create the DAL’s implementation.

Design of a ViewModel Based Data Access Pattern

I have been doing a lot of reading about data access pattern, and I haven’t really been happy with them. Since then I have been doing a lot of thinking about a good way to do data access.

The Repository Pattern

The repository pattern seems to be the most popular data access pattern. The problem I have with this pattern is that it is very rigid for querying. Generic repositories are pointless in my opinion as the business logic has to have intimate logic of the data layer in order to make efficient use of it, and if you are tying the two together you might as well deal with the data access manually in your business layer.

Non-generic repositories get tricky as you get into more complex querying scenarios. Most business processes your application will go through requires different information, even if the base entity it needs is the same. If you think about StackOverflow as an example, on one screen you need to query for the user’s information and all related questions but if you click on the “responses” link now you need the user’s information with all responses the user has made. The only way to do this without relying on lazy loading (which you don’t want as lazy loading can too easily become a performance nightmare) is to create your repository with one method per query. You will then end up with something like:

public interface IUserRepository
{
	User GetUserById(int id);
	User GetUserWithResponses(int id);
	User GetUserWithQuestions(int id);
	
	// etc...
}

Now say you need to implement a query that retrieves the user’s latest activity, which combines information from their questions and responses. You can’t user the previous two methods and you will have to implement yet another. This can very easily get out of hand as you require more and more types of queries. It also makes it confusing to determine which repository a query should be in (for example, if you want all the questions for a user is that in the User repository (user with associated questions) or in the questions repository (questions by user))?

Query Objects

Another pattern for querying is using Query Objects. I first learned about these from the 2nd half of this post by Ayende. The essence of this is that each class is for a single query, and to get the information you need you just call the Execute() method of the class required. While you now have a lot classes rather than a lot of methods, I find classes easier to organize and look for than repository methods.

The problem is that you still have to keep track of how you name your query classes, and remember exactly what data is returned by each query class. How do you distinguish between a query class that returns a user with his questions versus a query class that returns a user with his activity. It’s hard to do without keeping a very strict naming convention, and even then it can get a bit confusing.

Querying By ViewModel

Let’s stop thinking about the code for a second and think of the actual business/application logic of what we are trying to do. All we (or at least I) really want is to run a query to retrieve specific data. When working with my UI or business logic I don’t care about the specifics of how that happens, I just want the database to give me the information I need.

My idea is a slight extension of query objects. While each class is for one specific query, that class is not only for retrieving data from the database, but for placing that data in a specific non-persisted data class (aka a view model). The returned class doesn’t have to resemble the database’s data model at all, and thus this would give us total flexibility to retrieve data exactly as how our application wants it rather than relying on how the database stores it.

I would accomplish this via the following interface:

public interface IQuery<TViewModel>
{
	TViewModel Execute();
}

Now, let’s say I want to retrieve the user’s info with his questions. My UI might use the following view model

public class UserQuestionsPageViewModel
{
	public int UserId { get; set; }
	public string Name { get; set; }
	public IList<QuestionSummaryViewModel> Questions { get; set; }
}

The idea is that my Asp.Net MVC’s controller code to run this query would look like:

public class UserController : Controller
{
	protected IQuery<UserQuestionsPageViewModel> _query;
	
	public Usercontroller(IQuery<UserQuestionsPageViewModel> query)
	{
		_query = query;
	}

	public ActionResult UserQuestions(int userId)
	{
		var model = _query.Execute();
		return View(model);
	}
}

The interesting aspect of this is that we don’t care about how we named the query object, that part is already covered by dependency injection. All we have to specify is that we have a specific view model and we want to fill it with data from our data store, and the query commences. This makes our data access very simple and intuitive to use, and minimizes how many layers it takes to get data from the database into the view model for the UI.

Criteria

However, there is one weakness about this pattern. How do you specify your query criteria. In the previous example we still need some way to specify exactly which user we want to run the query for. I don’t know the best way to overcome this issue. best solution I have come up with so far is to have a super criteria object, that holds all the possible query criteria in one structure. An example of this would be:

public struct QueryCriteria
{
	public UserCriteria User { get; private set; }
	public QuestionCriteria Question { get; private set; }

	public struct UserCriteria
	{
		public int? ByIdNumber { get; set; }
		public string ByName { get; set; }
	}
	
	public struct QuestionCriteria
	{
		public int? ByIdNumber { get; set; }
		public string ByQuestioningUserId { get; set; }
	}
}

This structure would be created, the requested criteria would be set, and then it would be passed into the query’s Execute() method. The individual queries would then look for any criteria being set that’s relevant to them and retrieve the data using that criteria. If no criteria was set that the query is written to support it will throw an exception. One good thing about this method is that you now have one place where all query criteria can be found, which can give you a good reference for how queries are done in the system.

Final Thoughts

This is somewhat similar to a previous post I made, but after writing that post I realized the solution presented in that previous post was convoluted and needlessly complicated. This seems like a much overall better, and simpler, solution to allow many complicated queries in an organized fashion.

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
}