Thursday, July 28, 2011

Unit Test DisplayFormat attribute HtmlHelper

    Previously, I wrote about how to mock a ViewEngine for one of my unit tests. Today, I came across a question on StackOverflow asking about how to unit test the out put of DataAnnotation attribute DisplayFormat. I'm very interested in Unit Test so I decide to find the answer, another reason is to increase my reputation on StackOverflow :D. Honestly, I don't exactly know the reason why to test the output because for me, we don't need to test code that was not written by us. Anyways, this is a interesting question for me.     Let's say we have a ViewModel using DisplayFormat like below:
public class UserViewModel
{        
    [DisplayFormat(DataFormatString = "{0:dd/MM/yy}")]
    public DateTime Birthday { get; set; }
}
    And here is the basic unit test to verify the output
[Test]
public void Test_output_display_format_for_Birthday_property()
{
    // Arrange
    var _model = new UserViewModel {Birthday = DateTime.Parse("28/07/11") };
    var helper = MvcTestControllerBuilder.GetHtmlHelper<UserViewModel>();
    helper.ViewData.Model = _model;
    
    // Action
    var result = helper.DisplayFor(x => x.Birthday);
    
    // Assert
    Assert.That(result.ToHtmlString(), Is.EqualTo("28/07/11"));
}
    Apparently, this test failed like the description in the StackOverflow question. I decided to debug the MVC source code and found 2 reasons: + First, the MVC framework will find the ActionCacheItem using method GetActionCache:
internal static Dictionary<string, ActionCacheItem> GetActionCache(HtmlHelper html)
{
    HttpContextBase httpContext = html.ViewContext.HttpContext;
    if (!httpContext.Items.Contains(cacheItemId))
    {
        Dictionary<string, ActionCacheItem> dictionary = new Dictionary<string, ActionCacheItem>();
        httpContext.Items[cacheItemId] = dictionary;
        return dictionary;
    }
    return (Dictionary<string, ActionCacheItem>) httpContext.Items[cacheItemId];
}
It'll try to find the cache item in httpContext.Items but the Items is null. So the first thing we need to mock the value for httpContext.Items:
var helper = MvcTestControllerBuilder.GetHtmlHelper<UserViewModel>();
helper.ViewContext.HttpContext.Setup(x => x.Items).Returns(new Dictionary<string, object>());
+ Secondly, the MVC framework will try to find the display template for "DateTime" and "String". Obviously we don't have those stuff in the Unit test environment. The code is located in TemplateHelpers.cs:
internal static string ExecuteTemplate(HtmlHelper html, ViewDataDictionary viewData, string templateName, DataBoundControlMode mode, GetViewNamesDelegate getViewNames, GetDefaultActionsDelegate getDefaultActions)
{
    Dictionary<string, ActionCacheItem> actionCache = GetActionCache(html);
    Dictionary<string, Func<HtmlHelper, string>> dictionary2 = getDefaultActions(mode);
    string str = modeViewPaths[mode];
    foreach (string str2 in getViewNames(viewData.ModelMetadata, new string[] { templateName, viewData.ModelMetadata.TemplateHint, viewData.ModelMetadata.DataTypeName }))
    {
        ActionCacheItem item;
        Func<HtmlHelper, string> func;
        string key = str + "/" + str2;
        if (actionCache.TryGetValue(key, out item))
        {
            if (item != null)
            {
                return item.Execute(html, viewData);
            }
            continue;
        }
        ViewEngineResult result = ViewEngines.Engines.FindPartialView(html.ViewContext, key);
        if (result.View != null)
        {
            ActionCacheViewItem item2 = new ActionCacheViewItem {
                ViewName = key
            };
            actionCache[key] = item2;
            using (StringWriter writer = new StringWriter(CultureInfo.InvariantCulture))
            {
                result.View.Render(new ViewContext(html.ViewContext, result.View, viewData, html.ViewContext.TempData, writer), writer);
                return writer.ToString();
            }
        }
        if (dictionary2.TryGetValue(str2, out func))
        {
            ActionCacheCodeItem item3 = new ActionCacheCodeItem {
                Action = func
            };
            actionCache[key] = item3;
            return func(MakeHtmlHelper(html, viewData));
        }
        actionCache[key] = null;
    }
    throw new InvalidOperationException(string.Format(CultureInfo.CurrentCulture, MvcResources.TemplateHelpers_NoTemplate, new object[] { viewData.ModelMetadata.RealModelType.FullName }));
}
    So the main thing we need to do is somehow mock the result at line "19" to make the view engine return a ViewEngineResult that has View property equals to null. I make a helper method similar to this post to do this:
public static ViewEngineResult SetupNullViewFor(string viewName)
{
    Mock<IViewEngine> mockedViewEngine = GetCurrentMockViewEngine() ?? new Mock<IViewEngine>();
    var viewEngineResult = new ViewEngineResult(new List<string>());

    mockedViewEngine.Setup(x => x.FindPartialView(It.IsAny<ControllerContext>(), viewName, It.IsAny<bool>()))
                    .Returns(viewEngineResult);
    mockedViewEngine.Setup(x => x.FindView(It.IsAny<ControllerContext>(), viewName, It.IsAny<string>(), It.IsAny<bool>()))
                    .Returns(viewEngineResult);

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

private static Mock<IViewEngine> GetCurrentMockViewEngine()
{
    foreach(var v in ViewEngines.Engines)
    {
        try
        {
            return Mock.Get<IViewEngine>(v);
        }
        catch (Exception)
        {
        }
    }
    return null;
}
    With all of these stuff, the final unit test would look like:
[Test]
public void Test_output_display_format_for_Birthday_property()
{
    // Arrange    
    MvcTestFixtureHelper.SetupNullViewFor("DisplayTemplates/DateTime");
    MvcTestFixtureHelper.SetupNullViewFor("DisplayTemplates/String");
    
    var _model = new UserViewModel {Birthday = DateTime.Parse("28/07/11")};
    var helper = MvcTestControllerBuilder.GetHtmlHelper<UserViewModel>();
    helper.ViewContext.HttpContext.Setup(x => x.Items).Returns(new Dictionary<string, object>());
    helper.ViewData.Model = _model;
    
    // Action
    var result = helper.DisplayFor(x => x.Birthday);
    
    // Assert
    Assert.That(result.ToHtmlString(), Is.EqualTo("28/07/11"));
}
You can refer to my similar posts about MvcTestControllerBuilder (this class has some methods to mock HtmlHelper) and How to mock the ViewEngine. In those posts, I used NSubstitue but now I changed to use Moq, they're quite similar :D. Cheers.

Tuesday, July 19, 2011

Save some lines for your Razor view

We all know that we should not put logic in the MVC view. If we have to do something base on a complex condition, we should better make a HtmlHelper method for that logic. In an attempt to remove some simple if/else statement on the view by using some extention method. Let's say we have something like below in the view:
@if (!Request.IsAuthenticated)
{
    Html.RenderPartial("LoginBox");
}
I attempt to make it look like:
 
@{ Html.Do(x => x.RenderPartial("LoginBox"))
       .When(!Request.IsAuthenticated); }
 
So we can save 2 lines of code or event 3 lines if we write the code in 1 line:
 
@{ Html.Do(x => x.RenderPartial("LoginBox")).When(!Request.IsAuthenticated); }
 
I know it is very hard to remove all condition logic from the view unless you create HtmlHelper methods for all the condition logic on your view. But it could lead to so many methods. I think we should not create helper method if we don't use the method 2 places. So the method I suggest could help :D. I think it improves the readablity for your view code especially both the designers and developers are working on the same file. So here is the code:
public interface IAction<T>
{
    T Object { get;}
    Action<T> Action { get; }
    void When(Expression<Func<T, bool>> when);
    void When(bool when);
}
I will make 2 implementation of the above interface. The first one is OneConditionAction and the other is PostConditionAction. So I can combine multi conditions like:
@{ Html.When(!Request.IsAuthenticated)
       .And(....)
       .And(....)
       .Do(x => x.RenderPartial("LoginBox")); }
Below is the implementation:
public class OneConditionAction<T> : IAction<T>
{
    public T Object { get; private set; }

    public Action<T> Action { get; private set; }

    public virtual void When(Expression<Func<T, bool>> when)
    {
        if (when.Compile()(Object))
        {
            Action(Object);
        }
    }

    public void When(bool when)
    {
        if (when)
        {
            Action(Object);
        }
    }

    public OneConditionAction(Action<T> action, T helper)
    {
        Action = action;
        Object = helper;
    }
}       

public class PostConditionsAction<T> : OneConditionAction<T>
{
    public PostConditionsAction(IAction<T> conditionalAction)
        : base(conditionalAction.Action, conditionalAction.Object)
    {
        Condition = x => true;
    }

    public Expression<Func<T, bool>> Condition { get; set; }

    public override void When(Expression<Func<T, bool>> when)
    {
        var newCondition = this.And(when);
        Condition = newCondition.Condition;
    }
}
We'll need the extension methods:
namespace System.Web.Mvc
{
    public static class ActionExtensions
    {
        public static IAction<T> Do<T>(this T obj, Action<T> action) where T : class
        {
            return new OneConditionAction<T>(action, obj);
        }

        public static IAction<T> Do<T>(this T obj, Action action) where T : class
        {
            return new OneConditionAction<T>(a => action(), obj);
        }
    }
    
    public static class PostConditionsActionExtensions
    {
        public static PostConditionsAction<T> When<T>(this T obj, bool condition)
        {
            var newAction = new PostConditionsAction<T>(new OneConditionAction<T>(x => { }, obj));
            newAction = newAction.And(condition);
            return newAction;
        }

        public static PostConditionsAction<T> When<T>(this T obj, Expression<Func<T, bool>> condition)
        {
            var newAction = new PostConditionsAction<T>(new OneConditionAction<T>(x => { }, obj));
            newAction = newAction.And(condition);
            return newAction;
        }

        public static PostConditionsAction<T> And<T>(this PostConditionsAction<T> postConditions, Expression<Func<T, bool>> andCondition)
        {
            var x = Expression.Parameter(typeof(T));
            postConditions.Condition = Expression.Lambda<Func<T, bool>>(Expression.And(Expression.Invoke(postConditions.Condition, x), Expression.Invoke(andCondition, x)), x);
            return postConditions;
        }

        public static PostConditionsAction<T> And<T>(this PostConditionsAction<T> postConditions, bool andCondition)
        {
            return postConditions.And(x => andCondition);
        }

        public static PostConditionsAction<T> Or<T>(this PostConditionsAction<T> postConditions, Expression<Func<T, bool>> orCondition)
        {
            var x = Expression.Parameter(typeof(T));
            postConditions.Condition = Expression.Lambda<Func<T, bool>>(Expression.Or(Expression.Invoke(postConditions.Condition, x), Expression.Invoke(orCondition, x)), x);
            return postConditions;
        }

        public static PostConditionsAction<T> Or<T>(this PostConditionsAction<T> postConditions, bool orCondition)
        {
            return postConditions.Or(x => orCondition);
        }

        public static void Do<T>(this PostConditionsAction<T> postConditions, Action<T> action) where T : class
        {
            postConditions.Object.Do(action).When(postConditions.Condition);
        }

        public static void Do<T>(this PostConditionsAction<T> postConditions, Action action) where T : class
        {
            postConditions.Object.Do(action).When(postConditions.Condition);
        }
    }
}
Because they are generic classes, so we can use this syntax for any object, not only HtmlHelper :D Cheers

How to mock UrlHelper?

Sometime, you need to write unit test for an action method that needs a UrlHelper. But I'm sure you will never can mock that stuff since it's a concrete class and it's methods are not virtual. Let's say we have following action method and want to test it:
[Authorize]
public ActionResult LogOnAs(Guid userId, string returnUrl)
{
    // Logic to login user by Id ...
    
    if (Url.IsLocalUrl(returnUrl))
    {
        return Redirect(returnUrl);
    }

    return RedirectToAction("Index", "User");
}
I did some research, experimented with some results and find that extracting the UrlHelper methods to an interface then make a wrapper class to implement that interface is the best way. We need to define a new property to the BaseController like below.
 
public new IUrlHelper Url {get; set;}
 
Then in the unit test project, after initializing the controller, you can mock the interface easily. You probably need only some methods from the UrlHelper so I would recommend you extract only those required methods to IUrlHelper. For example:
public interface IUrlHelper
{
    string Action(string actionName, string controllerName);

    string Action(string actionName, string controllerName, object routeValues);

    string Action(string actionName, string controllerName, RouteValueDictionary routeValues);
    
    bool IsLocalUrl(string url);
}
Finally, we can create an adaptor class like below to delegate all method calls to the real UrlHelper object:
public class UrlHelperAdaptor : UrlHelper, IUrlHelper
{
    internal UrlHelperAdaptor(RequestContext requestContext)
        : base(requestContext)
    {
    }

    internal UrlHelperAdaptor(RequestContext requestContext, RouteCollection routeCollection)
        : base(requestContext, routeCollection)
    {
    }

    public UrlHelperAdaptor(UrlHelper helper) 
        : base(helper.RequestContext, helper.RouteCollection)
    {
    }
}
Apparently, we need to initialize the new Url property in the BaseController to make the real code work normally:
protected override void Initialize(System.Web.Routing.RequestContext requestContext)
{
    base.Initialize(requestContext);
    Url = new UrlHelperAdaptor(base.Url);
}
Now, the controller method is fully testable.
[Test]
public void LogonAs_should_return_RedirectToRouteResult()
{
    // Arrange
    var controller = new AccountController();
    /* Create a mock of IUrlHelper */
    controller.Url = Moq.Mock.Of<IUrlHelper>(x => x.IsLocalUrl(It.IsAny<string>()) == false);     

    // Action
    var result = controller.LogOnAs(Guid.NewGuid(), "any-return-url") as RedirectToRouteResult;

    // Assert
    result.Should().Not.Be.Null();
}
Using this approach can help you test any class that depends on IUrlHelper such as custom UrlHelper classes. Cheers

Wednesday, July 6, 2011

Never give a chance to make something wrong

Recectly, I've penetrated the truth that if you don't want others do something (bad), never ever give them a chance to do that. It's true in both my real life and in programming. Since my blog is just about software programming, I would like to write about some funny mistakes that people often make. Well, I did make some of them in the past :D

1/ You may forget about the ability of the constructor.

Let's look at following code:
public class UserViewModel
{    
    // These properties must not be null
    public string FirstName { get; set;}
    public string LastName { get; set;}
    public string Email { get; set;}    
}
We can see from the comment in the class above, the author want 3 propreties must have value. It oould be his intention but someone else may need a instance of this class, so he instantiates a UserViewModel object without setting the values for any of those 3 properties. And that could cause problems at runtime since it's not what the author of UserViewModel want. So instead of doing this, we can make the language ensuring that requirement for us:
using System.Diagnostics.Contracts;
public class UserViewModel
{    
    public UserViewModel(string firstName, string lastName, string email)
    {
        Contract.Assert(!string.IsNullOrEmpty(firstName), "firstName must have value");
        Contract.Assert(!string.IsNullOrEmpty(lastName) , "lastName must have value");
        Contract.Assert(!string.IsNullOrEmpty(email)    , "email must have value");
    
        FirstName = firstName;
        LastName = lastName;
        Email = email;
    }
    
    // These properties must not be null
    public string FirstName { get; private set;}
    public string LastName { get; private set;}
    public string Email { get; private set;}    
}

2/ You won't need a private field for a public property.

My team is applying BDD and using PageObject pattern. A guy in a team created a class that have a something like:
private RegistrationStatus _registrationStatus;

public RegistrationStatus Status
{
    get { return _registrationStatus; }
}
What an unnecessary 4 lines of code. You know what, just a few weeks later, the solution is full of these mistakes because alot of people are writing BDD tests and they like copy & paste. So, just change it to 1 line of code version:
 
public RegistrationStatus Status {get; private set;}
 
Well, if you don't want to see these stuff in the code, don't make any thing like this because we're all doing the "COPY & PASTE" job.

3/ If you have not finished something, use NotImplementedException

Apparently working in a team, someone could create a service interface with some methods then the other could use the service for presentation layer. Here is an example:
public interface IUserService
{
    User Get(Guid id);
    int Count();
}

public UserService : IUserService
{
    public User Get(Guid id)
    {
        return null;
    }
    
    public int Count()
    {
        return -1;
    }
}
Everything is fine when compile but why return something like those when they're useless value. Well, if we need an implementation of IUserService, why not create a Mock or a fake object then everyone knows it's a fake and it will never been used in the production. However, it could be acceptable if we are creating a NullObject, but I mostly throw NotImplementedException instead of doing nothing for a void method or return meaningless value for functions.

4/ it's not really a singleton

Everyone knows about singleton. It supposes to be the easiest most simple design pattern. Let's see this code:
public class Helper
{
    private static Helper _instance;
    public static Helper Instance
    {
        get
        {
            if (_instance == null)
            {
                _instance = new Helper();
            }
            return _instance;            
        }
    }
}
Okey, it's just a very simple implementation. But the idea is that it allows only 1 instance of Helper in the system and whenever you want that instance, you must access it from Helper.Instance. Cool but it's not really singleton. Anyone can create an object of Helper easily because by default, the constructor is public so make it private if you don't want people use it.

5/ We should follow Dependency Inversion Principle

This principle is the one that affects my way of thinking. If you read about the Onion architecture, you'll see that architecture is not complicated. It just applies the DIP and make a flashy name. If we apply DIP strickly, we'll never put implementation and interface together in a class library. Instead, put them in separated project. Classes depend on the interface will never know about the concreate implementation of that interface and they will be connected by some sort of IOC library. However, doing that way can make you create alot of projects in one solution. Honestly I have never done anything like this but, but I think Onion architecture is something to consider when you design new application. I prefer isolating the business implementation from the technology we use so any changes of technology will make a little affect to our code.

Wednesday, June 1, 2011

Basic Unit Of Work Implementation

    I bet you must have heard about Unit Of Work. If you have not, you can find the discription here : http://martinfowler.com/eaaCatalog/unitOfWork.html.    You can change the database with each change to your object model, but this can lead to lots of very small database calls, which ends up being very slow. Furthermore it requires you to have a transaction open for the whole interaction, which is impractical if you have a business transaction that spans multiple requests. The situation is even worse if you need to keep track of the objects you've read so you can avoid inconsistent reads.     Nowadays, DDD is one of the most famous design methodologies in the developer community. Even though 95% of all software applications fall into the “not so good for using DDD” categories, some practices and ideas of DDD can be used in many .NET projects, especially Repository and Domain Service. I have seen many implementations of Repository pattern and UnitOfWork, but i am not so happy with any because of the following reason. We tend to use some kinds of ORM like NHibernate or EF as the technology behind Repository implementation. These frameworks mostly have the Unit Of Work built in, so I think the Domain Service layer should not call IUnitOfWork.CommitChange() when it needs to persist something to the repository. In some complex services, we call a service method which may execute different service methods; and because a service method (that requires changing the database) should call IUnitOfWork.CommitChanges. Therefore, as a result, IUnitOfWork.CommitChanges will be called many times in 1 business transaction.     Another example could be the controller which can use several domain services to execute an action method. Let's say a website user wants to register at your website. When he submits the registration form, the RegisterController will call the UserService to create a user to the repository and another service create a WelcomeEmail in the Email repository. That means there are 2 records are got involed in this transaction (assume that you use SQL as the data source). And a proper UnitOfWork implementation should be called only 1 time to finish the saving task. However, it's easy to see the implementation like this:
[HttpPost]
public ActionResult Register(RegisterUserViewModel viewModel)
{
    if (!ModelState.IsValid)
    {
        return View(viewModel)
    }
    
    UserService.Create(user);
    EmailService.Create(welcomeEmail);
    
    return RedirectToAction("Index", "Home");
}
In the user service implementation :
public void Create(User contact)
{    
    _userRepository.Add(contact);
    _unitOfWork.CommitChange();
}
And in the email service implementation :
public void Create(Email email)
{    
    _emailRepository.Add(email);
    _unitOfWork.CommitChange();
}
    It's not difficult to see that the UnitOfWork will call method CommitChange() two times. If you use EF behind the Repository implementation, I'm sure the code above will make your breakpoint stop at method ObjectContext.SaveChanges() 2 times. For me it’s not “Unit Of Work” if we do like that. It would be nice if we could have a way to call the CommitChange() only when we actually need, it could be the end of every http request. Therefore, the domain services should never call CommitChange() on your UnitOfWork.     Base on this idea which is a bit similar to "Open Session In View" pattern, I created a basic implementation of UnitOfWork below:
public interface IUnitOfWork
{
    void CommitChange();
    bool IsDirty { get; set; }
}
The domain service will use a UnitOfWorkScope to ensure the change will be committed in the end or immediately based on the UnitOfWorkScopeOption:
public enum UnitOfWorkScopeOption
{
    Shared,
    CommitImmediately
}

public class UnitOfWorkScope : Disposable
{
    private readonly UnitOfWorkScopeOption _option;
    private readonly IUnitOfWork _unitOfWork;

    public UnitOfWorkScope() : this(UnitOfWorkScopeOption.Shared)
    {
    }

    public UnitOfWorkScope(UnitOfWorkScopeOption option)
    {
        _option = option;
        _unitOfWork = ObjectFactory.GetInstance<IUnitOfWork>(); // I'm using StructureMap
    }

    protected override void DisposeCore()
    {
        switch(_option)
        {
            case UnitOfWorkScopeOption.CommitImmediately:
                {
                    _unitOfWork.CommitChange();
                    _unitOfWork.IsDirty = false;
                    break;
                }
            default:
                _unitOfWork.IsDirty = true;
                break;
        }
    }
}
I will change the service methods like below:
public void Create(User contact)
{    
    using (new UnitOfWorkScope())
    {
        _userRepository.Add(contact);    
    }
}

public void Create(Email email)
{    
    using (new UnitOfWorkScope())
    {
        _emailRepository.Add(email);
    }
}
The default constructor of UnitOfWorkScope would set the scope option to Shared. That means it would do nothing but mark the UnitOfWork.IsDirty to true. In the end, we can base on that value to determine whether we should call commit change. I put this logic in the base controller, so after the ActionExecuted, the UnitOfWork will be invoked to do the saving job.
protected override void OnActionExecuted(ActionExecutedContext filterContext)
{
    base.OnActionExecuted(filterContext);

    if (filterContext.Exception == null || filterContext.ExceptionHandled)
    {
        var unitOfWork = ObjectFactory.GetInstance<IUnitOfWork>();
        if (unitOfWork != null && unitOfWork.IsDirty)
        {
            unitOfWork.CommitChange();
        }
    }    
}
I made a test project and looks like it run at least 2 times faster than the the original way when I use UnitOfWorkScope. There is a very cool implementation of UnitOfWork at NCommon. You should check it out. Finally, here is the test project. Cheers