MVC RouteConstraint is the culprite

Today, several of our live product sites got a serious issue. The page that renders a list of businesses took over 1 minutes to render. We couldn't find out the problem in serveral hours. After a while keep investigating, we found that the call of below method took about 6 seconds to run:

 
UrlHelper.Action("action", "controller", RouteValueCollection)
 

That means on a page that contains 10 links to the business detail would take about 1 minutes to render. It was really strange because this method is the built-in method of ASP.NET MVC so we didn't think it's our database issus. Another reason helped us beleave in our assumtion was that we tried to debug the service to get the list of businesses from database and saw that it was fast although we have over 1 mil of records in database, just the rendering step was so slow. I kept trying and stepped into MVC source code. It led me to this method which took 6 seconds to run:

 
RouteCollection.GetVirtualPath(RequestContext, RouteValueDictionary)
 

RouteCollection is a class in namespace System.Web.Routing and it was very strange because I couldn't continue to step into this method to debug. But anyways, it gave me another hint, there could be something wrong with our Routing table. So I opened the route registration code in our project and realized that we were using a RouteConstraint for the business route:

routes.MapRoute(
  "Business",
  "Business/{slug}",
  new { controller = "Business", action = "Detail" },
  new { slug = new BusinessSlugConstraint()}
);

That means whenever it renders the link like "/Business/business-slug" it will check whether the slug "business-slug" exists. We have over 1 mil of businesses in database so it could took a lot of time for this check. For now, what we can do to make a quick fix is rendering the detail links manually without using UrlHelper. It could save us some time and we'll address this issue later. Anyway, from now on, we should be careful when using the RouteConstraint cos it would never jump into it's code when we press F11 unless we put a break point in the RouteConstraint implementation. Cheers

Read more »

Optional Parameter Is Ugly

Optional Parameter has been introduced since C# 4.0. It's a nice feature indeed. Let's say we wanted to create a Html Helper method that can generate a Html markup of a link by providing the inner Html, controller and action name. The method would look like: HtmlLink(string innerHtml, string action, stirng controller) Definetily, there would be a requirement to provide a routeValue object to make the link in some cases like we want to have a link to edit a blog post, we would need something like: HtmlLink("Edit post", "Edit", "Blog", new {id = 1000}) Before C# 4.0, we had to create another overload for the above method. But with optional parameter we can easily archive this by adding the routeValue param as optional: HtmlLink(string innerHtml, string action, string controller, object routeValue = null) Cool, but the requirement would have a small change. We wanted to provide a css class name for the link so we had to change the method again: HtmlLink(string innerHtml, string action, string controller, object routeValue = null, object htmlAttribute = null) Hmmm, It's becoming more complicated but still okey because "everything is working fine" and "there is nothing wrong with it". Believe me, there are 2 common sentences that I hear everyday. Some days later, a guy in our team felt like it's time to make the method more handy by providing another optional parameter for area name. He wanted to make the change quickly without affecting the existing codes that have been using this method. So the method would be modified again: HtmlLink(string innerHtml, string action, string controller, string area = null, object routeValue = null, object htmlAttribute = null) Now, It's really a problem. The method had too many optional parameters. So when you want to use default area as null and provide route value object and htmlAttribute, then you have to use named arguments when call this method: HtmlLink("Edit post", "Edit", "Blog", routeValue: new {id = 1000}, htmlAttribute: new {@class = "edit-post"}) Given that we would use this powerful method to generate the links for some menu items. And there was a new requirement like if current page comes from the "menu item A" then this menu item should have css class "active". A guy who implemented that change could make this method even more "powerful" by adding another optional parameter: HtmlLink(string innerHtml, string action, string controller, string area = null, object routeValue = null, object htmlAttribute = null, bool isActive = false) It's not difficult to realize that this method's signature is so complicated due to the amount of parameters. It was cool at the first time but rapidly becomes ugly when being added more parameters. You would easily meet this kind of sittuation when you were in a team of many developers because there must be a time people want to have something done as quick as possible without thinking about clean code. I think optional parameter is good as soon as we use it in the reasonable way. For me, one optional parameter in a method is enough and 3 parameters should be the maximum of parameters in any methods. If we need more parameters, it's time to think about a DTO class.

Read more »

How to mock the MVC View Engine

Let's say we've created an HtmlHelper extension method to render a partial view without a need to provide a partial view name based on the type or template hint of a view model. The helper would look like:

public static void DisplayModel(this HtmlHelper htmlHelper, object model)
{
    var type = model.GetType();
    var modelMetadata = ModelMetadataProviders.Current.GetMetadataForType(null, type);
    if (string.IsNullOrEmpty(modelMetadata.TemplateHint))
    {
        htmlHelper.RenderPartial("DisplayTemplates/" + modelMetadata.ModelType.Name, model);
    }
    else
    {
        htmlHelper.RenderPartial("DisplayTemplates/" + modelMetadata.TemplateHint, model);
    }
}

This method is quite simple. It employs the built-in extension method of HtmlHelper: RenderPartial(). If you read the MVC source code, you will find that this method requires the ViewEngines to find the provided viewName and return a viewEngineResult. Then the viewEngineResult will be used to render the View content to Html. So it's quite complicated to test something that requires a ViewEngine behind the scene. I fell into this sittuation when i tried to test my extension methods (I attempt to raise the coverage for our team unit tests, started by writing unit tests for all the shits I made before.). After digging into the MVC source code, I think I need to replace the default view engines in ViewEngine.Engines collection by a mocked ViewEngine. Here is the implementation:

public static ViewEngineResult SetupViewContent(string viewName, string viewHtmlContent)
{
    var mockedViewEngine = Substitute.For<IViewEngine>();
    var resultView = Substitute.For<IView>();
    resultView.When(x => x.Render(Arg.Any<ViewContext>(), Arg.Any<TextWriter>())).Do(c =>
    {
        var textWriter = c.Arg<TextWriter>();
        textWriter.Write(viewHtmlContent);
    });
    var viewEngineResult = new ViewEngineResult(resultView, mockedViewEngine);

    mockedViewEngine.FindPartialView(Arg.Any<ControllerContext>(), viewName,  Arg.Any<bool>()).Returns(viewEngineResult);
    mockedViewEngine.FindView(Arg.Any<ControllerContext>(), viewName, Arg.Any<string>(), Arg.Any<bool>()).Returns(viewEngineResult);

    ViewEngines.Engines.Clear();
    ViewEngines.Engines.Add(mockedViewEngine);
    return viewEngineResult;
}

As i said in my previous posts, I'm using NSubstitute as my primary mock framework due to it's simplicity and easy to use. To setup all the things for my test, I'll create a mock for the IView which will be used to initialize the ViewEngineResult. And the ViewEngineResult will be the output for our mocked IViewEngine when it's method FindView or FindPartialView is executed. The most interesting point to do these stuff is that the default viewEngine collection in MVC3 has 2 item which are Aspx view engine and razor view engine. Therefore, to make our mocked view engine to be able to work, we need to clear these 2 default view engines and insert our mocked view engine. That's it. Here is one of the tests:

[TestMethod]
public void Can_use_a_model_type_as_partial_view_name_and_display_model_with_provided_view_model()
{
    // Arrange
    var builder = MvcTestControllerBuilder.GetDefaultBuilder();
    var htmlHelper = builder.GetHtmlHelper();
    htmlHelper.ViewContext.Writer = Substitute.For<StringWriter>();
    MvcTestFixtureHelper.SetupViewContent("DisplayTemplates/ProfileViewModel", "ProfileViewModel Content");

    // Action
    htmlHelper.DisplayModel(new ProfileViewModel());

    // Assert
    htmlHelper.ViewContext.Writer.Received().Write("ProfileViewModel Content");
}

Please refer to this post about MvcTestControllerBuilder. Cheers.

Read more »

What is the best unit test naming convention?

I'm not good at namming actually because I'm not an English native speaker. So I tried many best practices about naming the test methods and I met a few interesting strategies: - [Subject_Scenario_Result] or - [MethodName_StateUnderTest_ExpectedBehavior] And i found a very interesting way at: http://stevesmithblog.com/blog/unit-test-naming-convention/ For me I think those methods both have pros and cons. For the first one, in some complicated scenarios, my method name would look like: Edit_IfIdIsEmpty_ShouldCreateOrganisationAndReturnRedirectToRouteResult Sometime, when I want to write the expected behavior before the senario, I would write the name like: Edit_ShouldCreateOrganisationAndReturnRedirectToRouteResult_IfIdIsEmpty And time goes by, I have a lot of different styles of names and believe me, some of the names are very funny. I like ideas of the second method. When you run all your tests in the solution, the test result list will look beautiful and meanningful. But like the first method, in some complicated scenarioes, the name would be long and personally, I think it's hard to read and I'm not happy with the way he names the test class and test methods. Why don't we combine these methods and use this way: - Class name : ClassNameTests - Method name: Any_sentence_or_phrase_that_can_describe_the_intention_of_unit_test For example: BusinessServiceTests - Search_business_by_keyword_and_location_should_return_result_with_full_information UploadControllerTests - Should_throw_security_exception_if_file_type_is_not_supported I think using this way, we have the freedom of naming but still can provide the full intention of the unit test and the method name is very easy to read. Have a better idea? Please share it :D.

Read more »

Turn on Compile-time View Checking for ASP.NET MVC Projects for DEBUG only

I wrote a post about "Turn on Compile-time View Checking for ASP.NET MVC Projects" Today, we got a annoying problem with it. Our team have a gate checkin in TFS. Basically, it will build the solution when someone checks in code and if it builds successfully as well as all tests pass, the checkin will be approved. Some dude in our team did some code mergings and then checked in with so many errors on View files so we got serveral complains from the testers when they tested those pages. I expected my changes in the project configuration would not effect these builds on TFS because the MvcBuildViews would only be ignored on DEBUG configuration. And I also made an assumtion that the GateCheckin build on TFS didn't use DEBUG configuration but it did indeed. Actually, the GateCheckin was setup long time ago by someone and it was not set to choose a build configuration. So if you look in the top of the project file, you will see that if there is no configuration setup, the DEBUG will be default:

<Configuration Condition=" '$(Configuration)' == '' ">Debug</Configuration>
<Platform Condition=" '$(Platform)' == '' ">AnyCPU</Platform>

We raised this issue to the IT guy, just asked him to set the default build configuration for the GateCheckin because we don't have enough permission to change that. But there are not any responses... for some reasons :D. So I decided to cheat the build server myself. I change the line in the project configuration to something like:

<MvcBuildViews Condition=" '$(MvcBuildViews)' == '' Or '$(Configuration)' != 'Debug' ">true</MvcBuildViews>

The MSBUILD can get the property value from Environment Variables. That's how i cheat the build. The build server will never have MvcBuildViews in it's Environment Variables so it will have MvcBuildViews set to "true" always. In developer machines, we setup a system variable MvcBuildViews and set to false. This step will help skipping the view compilation while debugging, switch to Release or something else to compile the views when you want. Please note that it's neccessary to logoff then login or just restart the PC to make the environment variable take effect. Cheers

Read more »