Saturday, February 25, 2012

Write your own blog engine, part 2

Displaying posts

The next feature I'm going to work on is displaying the most recent five posts. The request isn't specific enough but I will assume that I should display them in descending order – most recent first, then the one before that and so on. I will further assume that each post will be contained inside an article tag and contain a date, title and post content.

You will be tempted to jump in and create a database and start thinking about the necessary fields. As weird as it sounds, it's far too soon. Yes, a database will be needed – the posts are data and they have to be persisted even when the application is not running – but we don't need it just yet. YAGNI, remember? You ain't gonna need it.

YAGNI means a bit more than what it says. It means: don't rush making decisions; you might need this but you might not; wait until the last responsible moment.

It is also tempting to put all the logic required – find the posts somehow, sort them in descending order by date, get the first five posts from the sorted list – inside the home controller. After all, that's the only production class we have so far. (I call them "production classes" by contrast with "test classes".) That can work but is not considered good practice. The single responsibility principle states that every class should have a single responsibility; the default responsibility of controller classes in MVC is to obtain a model object and pass it to a view. Think of it as a dispatcher. That means that figuring out how to obtain the model – in our case, the correct list of five posts – should be the responsibility of another class. Since the first rule I mentioned last time is that dependencies are exposed as interfaces, I will hide this new class behind an interface called PostRepository.

One of the interesting (and painful) things about TDD is that it forces good design – many small classes, many small methods, decoupling and so on. This is a big problem for new practitioners so they quickly abandon it when it seems like too much work. I just encountered a problem: how does the controller know about the repository? It will have to have a private field pointing to it, but how does it get that value? There are (at least) two methods: I could pass it in a public property or I could pass it in the constructor.

This pattern is called "dependency injection" and the two variants above are setter injection and constructor injection. Miško Hevery has a great article about the two styles and why he prefers constructor injection; I do too - property injection leaves open the possibility of incompletely constructed objects.

Time to write the new acceptance test. Because I'm testing the full application from the outside, as a black box, I cannot impose a specific return; all I can do is verify that some articles are being returned. I am going to use the article tag to mark each post, as I said. Here is the code:

    [TestMethod]
    public void HomePageReturnsMostRecentFiveArticles()
    {
      using (var web = new WebClient())
      {
        var html = web.DownloadString(BASE_URL);

        var doc = new HtmlDocument();
        doc.LoadHtml(html);
        var articles = doc.DocumentNode.SelectNodes("/html/body/article").ToList();
        Assert.IsTrue(articles.Any());
        var topArticle = articles.First();
        var header = topArticle.SelectSingleNode("header");
        Assert.IsNotNull(header);
        var title = header.SelectSingleNode("h2");
        Assert.IsNotNull(title);
        var date = header.SelectSingleNode("//time[@pubdate]");
        Assert.IsNotNull(date);
      }
    }

As you can see, there is some duplication in the acceptance tests, duplication that would be easily removed. ("Duplication is the enemy" is my mantra.) However, I will not refactor code if the tests aren't green - it's a good way to mess up everything and have to start over. Just something to keep in mind for after I make the new acceptance test pass.

The test fails because the SelectNodes call returns null, so the ToList() call crashes. That's fine for now; time to start working on the unit tests.

The first unit test should pass a PostRepository instance to the HomeController. Since I don't want to pass the real repository while testing (don't touch the database from the unit tests) I will have to pass a fake repository instead. You can write one by hand but I prefer to use the Moq framework, fortunately available as a NuGet package. Right-click the References folder in the MyBlogEngine.Tests project and choose "Manage NuGet Packages…". Make sure Online/All is selected on the left side and enter "moq" in the top right search box. Click "Install" on the Moq project to add the package to the project.

Here is the code for the unit test:

    [TestMethod]
    public void IndexCallsRepositoryToGetPosts()
    {
      var repository = new Mock<PostRepository>();
      var sut = new HomeController(repository.Object);

      sut.Index();

      repository.Verify(it => it.GetRecentPosts());
    }

Of course, this doesn't compile. Add a Services folder to the MyBlogEngine.MVC project and a new PostRepository interface inside that folder:

  public interface PostRepository
  {
    IEnumerable<Post> GetRecentPosts();
  }

Add a Post class in the Models folder:

  public class Post
  {
  }

As you can see, I'm keeping everything minimal until I actually need to work on it. Right now my focus is on making the code compile; adding properties to the Post class would not help with that goal so I don't do it.

The compiler is still complaining because of the parameter I'm passing to the HomeController constructor; add an empty constructor to silence it:

    public HomeController(PostRepository postRepository)
    {
    }

Now the first unit test is complaining; I'll just pass null to silence it, even though I know that's not going to work. Right now I'm trying to make the tests compile, not pass, so that's fine. By passing null everything compiles so now I can run the new unit test. It fails with the error "Expected invocation on the mock at least once, but was never performed: it => it.GetRecentPosts()". Perfect.

In order to fix this test I'm going to need to do something with the postRepository argument in the HomeController constructor. Here is the complete class:

  public class HomeController : Controller
  {
    public HomeController(PostRepository postRepository)
    {
      this.postRepository = postRepository;
    }

    public ActionResult Index()
    {
      postRepository.GetRecentPosts();

      return View();
    }

    //

    private readonly PostRepository postRepository;
  }

This thing with putting the private stuff at the end, after a separating comment line, is a personal quirk of mine. Feel free to use the more common style of putting the fields at the top of the class, or whatever style you prefer.

The new test passes. Two problems remain: nothing else does, and I "cheated" to make the test pass (I am calling the GetRecentPosts method but I don't do anything with the result). I'll handle the second problem first, by adding a new unit test:

    [TestMethod]
    public void IndexPassesPostsToView()
    {
      var model = new List<Post>();
      var repository = new Mock<PostRepository>();
      repository
        .Setup(it => it.GetRecentPosts())
        .Returns(model);
      var sut = new HomeController(repository.Object);

      var result = sut.Index() as ViewResult;

      Assert.AreEqual(model, result.Model);
    }

This test fails, of course, but that's easily fixed by changing the Index method:

    public ActionResult Index()
    {
      var posts = postRepository.GetRecentPosts();

      return View(posts);
    }

Good, the second problem has been solved, time to work on the first problem. I'll start by handling the first unit test:

    [TestMethod]
    public void IndexReturnsView()
    {
      var repository = new Mock<PostRepository>();
      var sut = new HomeController(repository.Object);

      var result = sut.Index() as ViewResult;

      Assert.IsNotNull(result);
    }

Since the unit tests are all passing now I am allowed to remove the duplication in them. Extract the creation of the repository and sut variables into an initialization method, since they are going to be used for all of the unit tests in the HomeControllerTests class:

  [TestClass]
  public class HomeControllerTests
  {
    [TestInitialize]
    public void SetUp()
    {
      repository = new Mock<PostRepository>();
      sut = new HomeController(repository.Object);
    }
    
    [TestMethod]
    public void IndexReturnsView()
    {
      var result = sut.Index() as ViewResult;

      Assert.IsNotNull(result);
    }

    [TestMethod]
    public void IndexCallsRepositoryToGetPosts()
    {
      sut.Index();

      repository.Verify(it => it.GetRecentPosts());
    }

    [TestMethod]
    public void IndexPassesPostsToView()
    {
      var model = new List<Post>();
      repository
        .Setup(it => it.GetRecentPosts())
        .Returns(model);

      var result = sut.Index() as ViewResult;

      Assert.AreEqual(model, result.Model);
    }

    //

    private Mock<PostRepository> repository;
    private HomeController sut;
  }

The unit tests are still passing so I didn't break anything; good.

The first acceptance test was passing before this change; it is failing now with a (500) Internal Server Error. message. In order to see why this is happening I am going to launch the website with Ctrl-F5 (I don't want to debug it, just see what's going on). I see right away what the problem is: No parameterless constructor defined for this object. Of course, the MVC engine doesn't know how to create a HomeController instance because the only constructor it finds is the one requiring a PostRepository.

I could solve this problem by adding a parameterless constructor that would create an actual implementation of the PostRepository interface; however, I don't yet have an implementation and I don't like the idea anyway (I would just be re-introducing the dependency on a concrete class the interface was meant to avoid). This means that I need to make MVC know how to create my controller. Not as hard as it sounds, fortunately, due to the existence of the Munq library. Right-click the References folder in the MyBlogEngine.MVC project and add the Munq.IocContainer and Munq.MVC3 packages (just like we did before with Moq, except that one was in the test project). This will add an App_Start folder and class; change that class to look like this:

  public static class MunqMvc3Startup
  {
    public static void PreStart()
    {
      DependencyResolver.SetResolver(new MunqDependencyResolver());

      var ioc = MunqDependencyResolver.Container;
      ioc.Register<PostRepository>(c => new DbPostRepository());
    }
  }

Add the DbPostRepository class to the Services folder:

  public class DbPostRepository : PostRepository
  {
    public IEnumerable<Post> GetRecentPosts()
    {
      return new List<Post>();
    }
  }

If I launch the web app now (Ctrl-F5) it starts correctly, so MVC has been able to create the home controller. Good. Running the tests shows me that the first acceptance test is also passing, leaving only the second one. Since I don't have any posts in a database, I will fake this for now by hard-coding one. Change the DbPostRepository.GetRecentPosts method to:

    public IEnumerable<Post> GetRecentPosts()
    {
      return new List<Post>
      {
        new Post { CreatedOn = DateTime.Now, Title = "Welcome", Content = "<p>This is a fake post.</p>" }
      };
    }

I've added some properties to the Post class:

  public class Post
  {
    public DateTime CreatedOn { get; set; }
    public string Title { get; set; }
    public string Content { get; set; }
  }

Finally, I need to do something to actually display the posts returned by the repository. Change the Index view to:

@{
    ViewBag.Title = "MyBlogEngine";
}

@using Renfield.MyBlogEngine.MVC.Models
@model IEnumerable<Post>

<h2>@ViewBag.Title</h2>

@foreach (var post in Model)
{
  <article>
    <header>
      <p><time pubdate="pubdate">@post.CreatedOn.ToLongDateString()</time></p>
      <h2>@post.Title</h2>
    </header>
    @Html.Raw(post.Content)
  </article>
}

All the tests are passing now, including the latest acceptance test; running the application allows me to verify that it is indeed working correctly (even though it looks very bland, as I've warned you from the start).

There's still one thing left to do: remove the duplication from the acceptance tests. Here is the modified class:

  [TestClass]
  public class AcceptanceTests
  {
    [TestInitialize]
    public void SetUp()
    {
      using (var web = new WebClient())
      {
        var html = web.DownloadString(BASE_URL);

        doc = new HtmlDocument();
        doc.LoadHtml(html);
        root = doc.DocumentNode;
      }
    }

    [TestMethod]
    public void HomePageHasCorrectTitle()
    {
      var title = root.SelectSingleNode("/html/head/title").InnerText;
      Assert.AreEqual("MyBlogEngine", title);
    }

    [TestMethod]
    public void HomePageReturnsMostRecentArticles()
    {
      var articles = root.SelectNodes("/html/body/article").ToList();
      Assert.IsTrue(articles.Any());
      var topArticle = articles.First();
      var header = topArticle.SelectSingleNode("header");
      Assert.IsNotNull(header);
      var title = header.SelectSingleNode("h2");
      Assert.IsNotNull(title);
      var date = header.SelectSingleNode("//time[@pubdate]");
      Assert.IsNotNull(date);
    }

    //

    private const string BASE_URL = "http://localhost:63516/";
    private HtmlDocument doc;
    private HtmlNode root;
  }

I have removed the word "five" from the name of the second acceptance test because that is not the proper place for it; I will have to make sure I'm limiting the number of posts returned in the DbPostRepository class.

I will attack the problem of authenticating the user in the next post.

Edit I do not normally use Internet Explorer, so it took me a while to discover that IE doesn't apply styles to tags it doesn't know about (and it doesn't know about HTML5, or at least not most versions actually used when I write this). That means you won't be able to make the views look better by changing the Site.css file, or at least you'll have problems with that. Fortunately, the same article told me what to do: just change the _Layout.cshtml file to this:

<!DOCTYPE html>
<html>
  <head>
    <meta charset="utf-8" />
    <title>@ViewBag.Title</title>
    <link href="@Url.Content("~/Content/Site.css")" rel="stylesheet" type="text/css" />
    <script src="@Url.Content("~/Scripts/jquery-1.5.1.min.js")" type="text/javascript"> </script>
    <script src="@Url.Content("~/Scripts/modernizr-1.7.min.js")" type="text/javascript"> </script>
    <script src="//html5shiv.googlecode.com/svn/trunk/html5.js" type="text/javascript"> </script>
  </head>

  <body>
    @RenderBody()
  </body>
</html>

Again, I am not normally using IE so don't take my word for it, verify that it does what it's supposed to do. I am not trying to create a pretty blog engine - I'm not even focusing on creating a blog engine, actually; my goal here is to use a blog engine as a pretext for showing how to apply TDD to a real project.

No comments: