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
}

Advertisements

Performing a fuzzy search with multiple terms through multiple Lucene.Net document fields

Recently I have been trying to implement Lucene.Net into my web application to allow users to search through their data. My core requirements were that the search allows minor mis-spellings and that search terms can can be found across different fields. The code to accomplish this wasn’t immediately obvious, and it took me a few days and a few questions posted to StackOverflow to finally figure out how to do it. Therefore, I thought it would be beneficial to others to show how to accomplish this.

To illustrate my scenario let’s assume I am searching for data indexed based on the following class:

public class Contact
{
    public string FirstName { get; set};
    public string LastName { get; set;};
}

When I index this structure in Lucene I decided to store the FirstName and LastName properties into separate fields in the Lucene document. This is to allow more advanced searching at a later time.

So for this scenario I want to allow a user to search for a contact with the name “Jon Doe”, but also allow the user to find the contact by using the search strings “John Doe”, “Jon”, or “Doe”.

The first thing that has to be done is to use a MultiFieldQueryParser() which allows a Lucene.net to search for the search terms in all of the specified fields of a document. The MultiFieldQueryParser can be used by the following code:

public SearchResults Search(string searchString)
{
            // Define fields in the document to search through
            string[] searchableFields = new string[] { "FirstName", "LastName" };
    
            // Generate the parser
            var parser = new MultiFieldQueryParser(Lucene.Net.Util.Version.LUCENE_29, searchfields, new                                                 StandardAnalyzer(Lucene.Net.Util.Version.LUCENE_29));
            parser.SetDefaultOperator(QueryParser.Operator.AND);

            // Perform the search
            var query = parser.Parse(searchString);
            var directory = FSDirectory.Open(new DirectoryInfo(LuceneIndexBaseDirectory));
            var searcher = new IndexSearcher(directory, true);
            var hits = searcher.Search(query, MAX_RESULTS);

            // Return results
}

The parser will split up all the terms of the search string and look for each term individually in all of the specified fields (I used an AND comparison with QueryParser.Operator.AND so that all search terms must be found for a document to be found). This code will allow me to perform successful searches with a search string of “Jon”, “Jon Doe”, and “Doe” but will not allow me to search for “John” as this method does not contain any code to allow fuzzy searches.

Implementing fuzzy search turned out to be the confusing part. The Lucene documentation’s only explanation for how to perform a fuzzy search is to add a tilde(~) to the end of a search term. With this knowledge, my first attempt was to add a tilde to the end of each word in the search query string. Unfortunately, it turns out that when Lucene is given multiple words in a search string and each word has a tilde at the end, a fuzzy search is only performed for the last word in the query, all others seem to be ignored and must be exact matches to get a hit.

After asking around it turns out that in order to allow each and every word to be non-exactly matched in the search you have to create a separate Lucene query for each term in the search string, and combine all the queries together using the BooleanQuery class. My implementation of this is:

public SearchResults Search(string searchString)
{
            // Setup the fields to search through
            string[] searchfields = new string[] { "FirstName", "LastName" };

            // Build our booleanquery that will be a combination of all the queries for each individual search term
            var finalQuery = new BooleanQuery();
            var parser = new MultiFieldQueryParser(Lucene.Net.Util.Version.LUCENE_29, searchfields, CreateAnalyzer());

            // Split the search string into separate search terms by word
            string[] terms = searchString.Split(new[] { " " }, StringSplitOptions.RemoveEmptyEntries);
            foreach (string term in terms)
                finalQuery.Add(parser.Parse(term.Replace("~", "") + "~"), BooleanClause.Occur.MUST);

            // Perform the search
            var directory = FSDirectory.Open(new DirectoryInfo(LuceneIndexBaseDirectory));
            var searcher = new IndexSearcher(directory, true);
            var hits = searcher.Search(finalQuery, MAX_RESULTS);
}

The BooleanClause.Occur.MUST ensures that all term queries must produce a match in order for the document to be a hit for the search.

The purpose of the term.Replace("~", "") + "~" code is to add a tilde to the end of the search term so Lucene knows to perform a fuzzy search with this term. We must remove any tildes the user entered in the search string to prevent an exception that will occur if a search term ends with two tildes (e.g. “John~~”).

This will successfully allow you to perform a fuzzy search with multiple terms across multiple document fields. The algorithm does eventually need to be improved to not split out search terms that are inside of quotations (to signify a search for a phrase) but I hope this helps others figure out fuzzy searching more quickly than it took me.

Allowing Eager Loading In Business Logic via View Models

Today I am going to write a post about a library I have been thinking about for the past week. If I end up actually developing it I plan to make it open source as I think it solves an issue that a lot of systems may encounter in regards to eager loading database entities, and dealing with a separation of the data entities from the domain model. I am going to use this post to bring all of my thoughts together to make sure what’s in my head actually makes sense, as well as to get any feedback (if this is actually read by anyone but me)

The Problem

My web application is currently using the Entity Framework 4.1 CodeFirst system. The original issue came about when I was looking at document-oriented databases and the possibility of switching my backend to RavenDB. It wasn’t a serious idea, as it would be a huge hassle to convert the production data from sql server to a document oriented database for little benefit (at this time at least). However, it did get me to realize that even though my architecture allows me to change database backends and ORMs pretty painlessly, it does not allow me to change my data model without changing my business entity model to match. This is mostly due to my use of Linq to specify queries in my business layer.

My thoughts then went to switching to use a repository pattern for data access. The repository pattern would allow me to more easily organize my queries, abstract away data access from the business layer, and would mean that my data model could be completely different from my business entity model and changes can be made to one without affecting the other. Unfortunately, the repository pattern has a big issue when it comes to eager loading in that it usually ends up with many repeating methods, such as:

public class EFUserRepository : IUserRepository
{
    public User GetUserById(int id) { }
    public User GetUserByIdWithContacts(int id) { }
    public User GetUserByIdWithContactsWithCompanies(int id) { }
    ....
}

I have not found a better solution to handle eager loading with the repository pattern and the previous examples has too much repeating code for my liking.

The VMQA (View Model Querying Assistant) Library

After thinking about this problem for a bit, I later realized that what I ultimately want is to pass a view model data structure into a repository query method, for the repository to come up with (and execute) an eager loading strategy based on the view model, and then to map all the properties of the view model with the data retrieved from the database. To accomplish this I want to rewrite my repository methods to look something like:

public class EFUserRepository : IUserRepository
{
    public T GetUserById<T>(int id) where T : class 
    { 
        // Form the query
        var query = _context.Users.Where(x => x.Id == id);

        // Determine the eager loading strategy
        var includeStrings = vmqa.GetPropertiesToEagerLoadForClass(T).ConvertToEFStrings();
        foreach (string inc in includeStrings)
            query.Include(inc);

        // Execute the query and map results into the specified data structure
        var user = query.First();

        // Return an instance of the view model with the data filled in from the user
        return vmqa.MapEntityToViewModel<User, T>(user);
    }
}

The design I have floating around in my head to accomplish this has two parts.

Eager Loading

The first part is coming up with an eager loading strategy based on the specified data structure, a data structure which the library has no previous knowledge of. I envision this working by building a map of how all the data entities fit together with an interface that is similar to how configuring EF CodeFirst models is done. An example mapping would be:

public void MapEntities()
{
    vmqa.Entity<User>().HasMany(typeof(Contact), x => x.ContactsProperty);
    vmqa.Entity<Contact>().HasOne(typeof(Company), x => x.CompanyProperty);
}

This would tell the system that the User entity maps to Contacts via the User.ContactsProperty.

In my repository I would then call upon the VMQA library to look at the data structure and resolve the eager loading strategies. To allow the VMQA library to understand how to map an arbitrary view model data structure to a data entity, I would use custom attributes that would decorate a class such as:

[MapsToEntity(typeof(User))]
public class UserViewModel
{
    public int Id { get; set; }
    public string Name { get; set; }

    [MapsToEntity(typeof(Contact))]
    public IList Contacts { get; set; }
}

Using reflection, the vmqa.GetPropertiesToEagerLoadForClass should be able to figure out that the core entity maps to the User, that Contacts collection maps to the Contact. Since it knows that the view model explicitly specifies a view model for the Contacts entity it can determine that the Contacts property needs to be eager loaded, and will then return a data structure containing the Contact type and the name of the property it maps to from the User entity.

It should be easy to allow this system to allow complex mappings, so if the user view model has a property that maps to a collection of companies, the system can automatically find a path from User to for eager loading.

The ConvertToEFStrings() extension method would then convert that data structure into a collection of strings that Entity Framework can use for eager loading. This would be an extension method so that the library is not restricted to just Entity Framework, and can be used for any ORM or database system.

Entity Mapping

The second part of the whole system deals with automatically mapping the returned data entities into the view model. This seems like a natural extension of the library to eager loading to me, as if you are passing the view model into the repository to describe what data you want to retrieve, it should automatically copy that data into the view model for your usage. No matter what, the developer is going to want to do this anyway and it seem sensible to automatically do this with VMQA, since it already has knowledge about the relationships.

My first version of the library would probably do a straight name for name mapping, so that UserViewModel.Name maps to User.Name. Eventually I would probably add property mapping attributes so more complex mappings can be performed.

Conclusion

After finally getting this design written down, I feel more confident than ever that this would be an extremely valuable library to have, and that it actually seems relatively straight-forward to create. Now I just need the time!

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.