Sunday, February 26, 2012

Write your own blog engine, part 4

Adding posts

I finally arrive at the "raison d'être" of a blog: adding posts. I want to be able to add posts and have them be returned in the next GET call to the home page. I also want to be able to add posts out of order by explicitly indicating the date/time of the new post (so that it gets sorted somewhere else than the top position).

I'll start with an acceptance test for the first part:

    [TestMethod]
    public void AddingAPostReturnsItOnTheHomePage()
    {
      var guid = Guid.NewGuid();
      var POST_DATA = string.Format("Title={0}&Content=This is a test", guid);

      var cookie = Post("/Account/LogOn", "Username=user&Password=pass");
      Post("/Home/AddPost", POST_DATA, cookie);
      Get();

      var articles = root.SelectNodes("/html/body/article").ToList();
      Assert.IsTrue(articles.Any());
      var topArticle = articles.First();
      var header = topArticle.SelectSingleNode("header");
      var title = header.SelectSingleNode("h2");
      Assert.AreEqual(guid.ToString(), title.InnerText);
    }

The private Post method looks like this:

    private static string Post(string relativeUrl, string data, string cookie = null)
    {
      using (var web = new MyWebClient())
      {
        web.Headers["Content-Type"] = "application/x-www-form-urlencoded";
        if (cookie != null)
          web.Headers[HttpRequestHeader.Cookie] = cookie;

        web.UploadString(BASE_URL + relativeUrl, data);

        return web.ResponseHeaders["Set-Cookie"];
      }
    }

Note that this test is not idempotent; that is, it doesn't leave the system in the same way it found it. Every time this test runs successfully a new post is created. That's why I'll mark it with the [Ignore] attribute after I manage to make it pass.

The test fails with a "The remote server returned an error: (404) Not Found." error because there's no AddPost method on the HomeController. Time to start working on the unit tests, the first of which will verify that new posts are being added to the repository:

    [TestMethod]
    public void POST_AddPostAddsToTheRepository()
    {
      var post = new Post();

      sut.AddPost(post);

      repository.Verify(it => it.AddPost(post));
    }

Of course, this means adding a new method to the PostRepository interface:

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

with the corresponding (empty) implementation on the DbPostRepository class:

    public void AddPost(Post post)
    {
    }

and finally the AddPost method on the HomeController class:

    [HttpPost]
    public ActionResult AddPost(Post post)
    {
      return null;
    }

Of course, the unit test fails with an error: "Expected invocation on the mock at least once, but was never performed: it => it.AddPost(.post)". This means I have to change the AddPost method to:

    [HttpPost]
    public ActionResult AddPost(Post post)
    {
      postRepository.AddPost(post);

      return null;
    }

Don't get over-excited and try to write the full implementation of the method. The majority of the lifetime of a project is spent reading and modifying existing code, not writing it for the first time. That means that when you or someone else will come back six months later to make a change to this code you will need a way to make sure that the changes you make don't break features. If you just write the code without tests you won't have that "safety net" to prevent you from making the wrong changes.

Ok, this method is clearly under-specified. What should happen after a post has been successfully added? I decided that the customer (me) will want to see that the new post has been added so I'll return to the home page:

    [TestMethod]
    public void POST_AddPostRedirectsToTheHomePage()
    {
      var result = sut.AddPost(new Post()) as RedirectToRouteResult;

      Assert.AreEqual("Index", result.RouteValues["action"]);
    }

Making it pass is easy:

    [HttpPost]
    public ActionResult AddPost(Post post)
    {
      postRepository.AddPost(post);

      return RedirectToAction("Index");
    }

Good. Time to work on persistence, more precisely on the (falsely named so far) DbPostRepository class. I am going to use the EntityFramework package for this project because I love its code-first features; this package is installed by default by the MVC applications but you should make sure it is up to date: right click the References folder, choose "Manage NuGet packages…", click on Updates on the left side and, if EntityFramework does show up in the list, click Update.

Now that this has been taken care of I will create a new class inheriting from DbContext; this class will be "the database" as far as the code is concerned. Add a new Persistence folder to the MyBlogEngine.MVC project and a new BlogDB class to it:

  public class BlogDB : DbContext
  {
    public DbSet<Post> Posts { get; set; }
  }

The DbPostRepository class will receive an instance of the BlogDB class in its constructor… wait, no it won't, that would break the first rule I decided on: only depend on abstractions, never on concrete classes.

This is called the Dependency Inversion Principle - this article (pdf) has a very detailed explanation of it.

That means I'm going to need an interface; fortunately, EF 4 (at least) was designed to be testable so that's not very difficult to do. Here are the BlogPersistence interface and the updated BlogDB class:

  public interface BlogPersistence
  {
    IDbSet<Post> Posts { get; }

    int SaveChanges();
  }

  public class BlogDB : DbContext, BlogPersistence
  {
    public IDbSet<Post> Posts { get; set; }
  }

Now the DbPostRepository class can require a BlogPersistence argument in its constructor and thus allow testing without actually hitting the database. That's good, let's start by writing some of those tests. The first one should make sure that a fake post will be returned if there are no real posts in the database (so as not to break the second acceptance test), so here is the new DbPostRepositoryTests class:

  [TestClass]
  public class DbPostRepositoryTests
  {
    [TestMethod]
    public void GetRecentPostsReturnsFakePostIfTableIsEmpty()
    {
      var persistence = new Mock<BlogPersistence>();
      var sut = new DbPostRepository(persistence.Object);

      var result = sut.GetRecentPosts().ToList();

      Assert.AreEqual(1, result.Count);
      Assert.AreEqual("Welcome", result[0].Title);
    }
  }

I'll have to add a constructor to make it compile:

    public DbPostRepository(BlogPersistence blogPersistence)
    {
    }

This in turn breaks down the MunqMvc3Startup class so I'll change the PreStart method:

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

      var ioc = MunqDependencyResolver.Container;
      ioc.Register<BlogPersistence>(c => new BlogDB());
      ioc.Register<PostRepository>(c => new DbPostRepository(c.Resolve<BlogPersistence>()));
      ioc.Register<UserService>(c => new HardcodedUserService());
    }

The test is… passing. Oops. If a new test doesn't start by failing something is wrong; in this particular case the test is passing for the wrong reason - I'm always returning the fake post. I'm tempted to skip over the intermediary step and just write the correct implementation but a bit of discipline can't hurt. I replace the method body with a single return new List<Post>(); line and verify that it is indeed failing (Assert.AreEqual failed. Expected:<1>. Actual:<0>.). Now I can go ahead and write the correct implementation, which requires changing more than just that method:

  public class DbPostRepository : PostRepository
  {
    public DbPostRepository(BlogPersistence blogPersistence)
    {
      this.blogPersistence = blogPersistence;
    }

    public IEnumerable<Post> GetRecentPosts()
    {
      var posts = blogPersistence.Posts.ToList();
      if (!posts.Any())
        posts.Add(GetFakePost());

      return posts;
    }

    public void AddPost(Post post)
    {
    }

    //

    private readonly BlogPersistence blogPersistence;

    private static Post GetFakePost()
    {
      return new Post { CreatedOn = DateTime.Now, Title = "Welcome", Content = "<p>This is a fake post.</p>" };
    }
  }

The test is now… failing. (This stuff is hard.) That's because blogPersistence.Posts is null so the ToList call crashes. Ok, I need to fix the test to return an empty list:

    [TestMethod]
    public void GetRecentPostsReturnsFakePostIfTableIsEmpty()
    {
      var postsTable = new Mock<IDbSet<Post>>();
      postsTable
        .Setup(it => it.GetEnumerator())
        .Returns(new List<Post>().GetEnumerator());
      var persistence = new Mock<BlogPersistence>();
      persistence
        .SetupGet(it => it.Posts)
        .Returns(postsTable.Object);
      var sut = new DbPostRepository(persistence.Object);

      var result = sut.GetRecentPosts().ToList();

      Assert.AreEqual(1, result.Count);
      Assert.AreEqual("Welcome", result[0].Title);
    }

All of the unit tests are passing now. Whew. However, when I said before that I was writing the correct implementation I was exaggerating a bit. The reason for that is that I'm not restricting the posts being returned in any way: I will happily return hundreds of posts from this method. I'll write a test to expose that problem:

    [TestMethod]
    public void GetRecentPostsOnlyReturnsFive()
    {
      var posts = new List<Post>();
      for (var i = 0; i < 10; i++)
        posts.Add(new Post());
      var postsTable = new Mock<IDbSet<Post>>();
      postsTable
        .Setup(it => it.GetEnumerator())
        .Returns(posts.GetEnumerator());
      var persistence = new Mock<BlogPersistence>();
      persistence
        .SetupGet(it => it.Posts)
        .Returns(postsTable.Object);
      var sut = new DbPostRepository(persistence.Object);

      var result = sut.GetRecentPosts().ToList();

      Assert.AreEqual(5, result.Count);
    }

This test fails (Assert.AreEqual failed. Expected:<5>. Actual:<10>.). That's ok, I can fix that easily:

    public IEnumerable<Post> GetRecentPosts()
    {
      var posts = blogPersistence.Posts.ToList();
      return posts.Any()
               ? posts.Take(5)
               : new List<Post> { GetFakePost() };
    }

Of course, returning any five posts isn't enough; I need to return the most recent five. That means another test:

    [TestMethod]
    public void GetRecentPostsReturnsTheMostRecentFive()
    {
      var posts = new List<Post>();
      for (var i = 0; i < 10; i++)
        posts.Add(new Post { CreatedOn = new DateTime(2000, 1, i + 1), Title = i.ToString() });
      var postsTable = new Mock<IDbSet<Post>>();
      postsTable
        .Setup(it => it.GetEnumerator())
        .Returns(posts.GetEnumerator());
      var persistence = new Mock<BlogPersistence>();
      persistence
        .SetupGet(it => it.Posts)
        .Returns(postsTable.Object);
      var sut = new DbPostRepository(persistence.Object);

      var result = sut.GetRecentPosts().ToList();

      Assert.AreEqual("9", result[0].Title);
      Assert.AreEqual("8", result[1].Title);
      Assert.AreEqual("7", result[2].Title);
      Assert.AreEqual("6", result[3].Title);
      Assert.AreEqual("5", result[4].Title);
    }

Again, easy to fix. (One of the important advantages of TDD is that you're supposed to write code in small steps: easy to write test, easy to write code to make it pass. If something is difficult to write it usually indicates a problem, either with the existing design or with your understanding of the domain.)

    public IEnumerable<Post> GetRecentPosts()
    {
      var posts = blogPersistence.Posts.ToList();
      return posts.Any()
               ? posts
                   .OrderByDescending(p => p.CreatedOn)
                   .Take(5)
               : new List<Post> { GetFakePost() };
    }

The new unit test passes too and so do all others. (Not the acceptance tests though - something is wrong there, but it's not yet time to look into that.)

Now that the GetRecentPosts method is fully specified, it's time to look at the test code. There's far too much duplication for my taste. (I initially wrote "test" instead of "taste". Har, har.) The modified DbPostRepositoryTests class looks like this:

  [TestClass]
  public class DbPostRepositoryTests
  {
    [TestInitialize]
    public void SetUp()
    {
      posts = new List<Post>();
      postsTable = new Mock<IDbSet<Post>>();
      persistence = new Mock<BlogPersistence>();
      persistence
        .SetupGet(it => it.Posts)
        .Returns(postsTable.Object);
      sut = new DbPostRepository(persistence.Object);
    }

    [TestMethod]
    public void GetRecentPostsReturnsFakePostIfTableIsEmpty()
    {
      SetUpPosts();

      var result = sut.GetRecentPosts().ToList();

      Assert.AreEqual(1, result.Count);
      Assert.AreEqual("Welcome", result[0].Title);
    }

    [TestMethod]
    public void GetRecentPostsOnlyReturnsFive()
    {
      for (var i = 0; i < 10; i++)
        posts.Add(new Post());
      SetUpPosts();

      var result = sut.GetRecentPosts().ToList();

      Assert.AreEqual(5, result.Count);
    }

    [TestMethod]
    public void GetRecentPostsReturnsTheMostRecentFive()
    {
      for (var i = 0; i < 10; i++)
        posts.Add(new Post { CreatedOn = new DateTime(2000, 1, i + 1), Title = i.ToString() });
      SetUpPosts();

      var result = sut.GetRecentPosts().ToList();

      Assert.AreEqual("9", result[0].Title);
      Assert.AreEqual("8", result[1].Title);
      Assert.AreEqual("7", result[2].Title);
      Assert.AreEqual("6", result[3].Title);
      Assert.AreEqual("5", result[4].Title);
    }

    //

    private List<Post> posts;
    private Mock<IDbSet<Post>> postsTable;
    private Mock<BlogPersistence> persistence;
    private DbPostRepository sut;

    private void SetUpPosts()
    {
      postsTable
        .Setup(it => it.GetEnumerator())
        .Returns(posts.GetEnumerator());
    }
  }

All the unit tests are still passing so I didn't break anything.

Time to see what's wrong with the acceptance tests. The easiest way is to try to run the application and I immediately get back an error: "EntityType 'Post' has no key defined." True - EF requires that all entities have a primary key. I've decided to use a GUID as the primary key for the Posts instead of an integer or something else; making this change fixes all the acceptance tests except for the last one:

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

Note that this made EF create the database because I have SQL Express installed and EF will create the database there if it can. This is how the Posts table was created:

CREATE TABLE [dbo].[Posts](
[Id] [uniqueidentifier] NOT NULL,
[CreatedOn] [datetime] NOT NULL,
[Title] [nvarchar](max) NULL,
[Content] [nvarchar](max) NULL,
 CONSTRAINT [PK_Posts] PRIMARY KEY CLUSTERED 
(
[Id] ASC
)WITH (PAD_INDEX  = OFF, STATISTICS_NORECOMPUTE  = OFF, IGNORE_DUP_KEY = OFF, ALLOW_ROW_LOCKS  = ON, ALLOW_PAGE_LOCKS  = ON) ON [PRIMARY]
) ON [PRIMARY]

The last acceptance test fails because the DbPostRepository.AddPost method doesn't actually do anything. What it should do is, of course, add the new post to the database so I'll write a test to specify that:

    [TestMethod]
    public void AddPostAddsTheNewPostToTheDatabase()
    {
      var post = new Post();

      sut.AddPost(post);

      postsTable.Verify(it => it.Add(post));
      persistence.Verify(it => it.SaveChanges());
    }

The specification is so detailed that writing the production code is practically automatic:

    public void AddPost(Post post)
    {
      blogPersistence.Posts.Add(post);
      blogPersistence.SaveChanges();
    }

This made the unit test pass but now the acceptance test fails with an internal error. That's because the CreatedOn field was not specified. I should default it to the current date/time so here's a test verifying that:

    [TestMethod]
    public void AddPostDefaultsCreatedOnToCurrentDateTime()
    {
      var post = new Post();

      sut.AddPost(post);

      Assert.AreEqual(DateTime.Now, post.CreatedOn);
    }

Easy to make it pass:

    public void AddPost(Post post)
    {
      if (post.CreatedOn == DateTime.MinValue)
        post.CreatedOn = DateTime.Now;

      blogPersistence.Posts.Add(post);
      blogPersistence.SaveChanges();
    }

Except… the test fails because it takes a bit of time between the end of the AddPost method call and the test and thus the value of DateTime.Now has changed. Well, I didn't like having a dependency on DateTime anyway. Here's how to fix it; change the test to:

    [TestMethod]
    public void AddPostDefaultsCreatedOnToCurrentDateTime()
    {
      var post = new Post();
      var dt = new DateTime(2000, 1, 1);
      sut.Clock = () => dt;

      sut.AddPost(post);

      Assert.AreEqual(dt, post.CreatedOn);
    }

and add this to the beginning of the DbPostRepository class:

    public Func<DateTime> Clock = () => DateTime.Now; 

Of course, the AddPost method must change to use the new Clock function instead of DateTime.Now:

    public void AddPost(Post post)
    {
      if (post.CreatedOn == DateTime.MinValue)
        post.CreatedOn = Clock();

      blogPersistence.Posts.Add(post);
      blogPersistence.SaveChanges();
    }

All the tests pass, including the acceptance tests and, in addition to that, running the application no longer returns the fake post (it does return a test post instead). Unfortunately, looking at the table in the database I discover that the Id of the test post is the empty GUID (all zeroes) and, in fact, if I run the last acceptance test I am now getting an internal error. That means I need to create a new GUID in the AddPost method if the post's key is the empty GUID. I'll start with a test:

    [TestMethod]
    public void AddPostDefaultsTheIdToANewGuid()
    {
      var post = new Post();

      sut.AddPost(post);

      Assert.AreNotEqual(Guid.Empty, post.Id);
    }

I could have gone the same route as before and extract the Guid.NewGuid call into a Func<Guid> delegate but I chose not to. I don't care what value the Id gets as long as it's not the empty GUID.

The final fix to the AddPost method looks like this:

    public void AddPost(Post post)
    {
      if (post.CreatedOn == DateTime.MinValue)
        post.CreatedOn = Clock();
      if (post.Id == Guid.Empty)
        post.Id = Guid.NewGuid();

      blogPersistence.Posts.Add(post);
      blogPersistence.SaveChanges();
    }

All the tests are passing and I can now mark the last acceptance test with the [Ignore] attribute. I will have to make sure to re-run that test once in a while but for the most part I'm satisfied with it being disabled.

I'm still not done, of course: I need a way for the user to add the new post and I need to make sure only authenticated users can do that. Here's the test for the GET call to the AddPost method:

    [TestMethod]
    public void GET_AddPostReturnsView()
    {
      var result = sut.AddPost() as ViewResult;

      Assert.AreEqual("", result.ViewName);
    }

The new method is very simple:

    [HttpGet]
    public ActionResult AddPost()
    {
      return View(new Post());
    }

The view looks like this:

@{
  ViewBag.Title = "New post";
}

@using Renfield.MyBlogEngine.MVC.Models
@model Post

<h2>@ViewBag.Title</h2>

@Html.ValidationSummary()

@using(Html.BeginForm())
{
  <fieldset>
    <legend>Add new post</legend>
    @Html.EditorForModel()

    <input type="submit" value="Submit"/>
  </fieldset>
}

Two things remain to be done: mark the two AddPost method with the [Authorize] attribute so that only authenticated users can call them (non-authenticated users get redirected to the LogOn method) and add a link to the GET AddPost method in the home page view. I just changed the Index view to:

@{
    ViewBag.Title = "MyBlogEngine";
}

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

<h2>@ViewBag.Title</h2>

@Html.ActionLink("Create new post", "AddPost")

@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>
}

Hmm… I'm still not done. For one thing, clicking on the new "Create new post" link does send me to the LogOn page, but after I log on correctly it sends me back to the home page. That's because I was looking for the return url in the ViewData property and instead it was sent in the query string. I will have to change a test to account for that:

    [TestMethod]
    public void POST_LogOnRedirectsToPreviousPageForValidCredentials()
    {
      const string RETURN_URL = "url";
      var user = new User { Username = "x", Password = "y" };
      var cookies = new HttpCookieCollection();
      var service = new Mock<UserService>();
      service
        .Setup(it => it.ValidateUser(user, cookies))
        .Returns(true);
      var sut = new AccountController(service.Object);
      sut.SetFakeControllerContext();
      sut.Request.SetupRequestUrl("~/?ReturnUrl=" + RETURN_URL);
      sut.Response.SetUpCookies(cookies);

      var result = sut.LogOn(user) as RedirectResult;

      Assert.AreEqual(RETURN_URL, result.Url);
    }

The POST LogOn method changes to:

    [HttpPost]
    public ActionResult LogOn(User user)
    {
      var returnUrl = Request.QueryString["ReturnUrl"];

      if (userService.ValidateUser(user, Response.Cookies))
      {
        return returnUrl != null
                 ? (ActionResult) Redirect(returnUrl)
                 : RedirectToAction("Index", "Home");
      }

      ModelState.AddModelError("", "Invalid user or password");

      return View(user);
    }

That makes the test pass but unfortunately two of the other tests in the AccountControllerTests class fail because QueryString is null. The fix is to change the beginning of the POST LogOn method to

    var returnUrl = Request.QueryString != null ? Request.QueryString["ReturnUrl"] : null;

This makes all the tests pass.

The remaining issue is the AddPost view: the Id field should not be visible and the CreatedOn field should contain the current date/time. Change the Post class like this to fix the first issue:

  public class Post
  {
    [HiddenInput(DisplayValue = false)]
    public Guid Id { get; set; }

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

The second issue has to be fixed inside the HomeController class, which means a new unit test:

    [TestMethod]
    public void GET_AddPostInitializesTheCreatedOnField()
    {
      var dt = new DateTime(2000, 1, 1);
      sut.Clock = () => dt;

      var result = sut.AddPost() as ViewResult;

      Assert.AreEqual(dt, ((Post) result.Model).CreatedOn);
    }

Just like I did earlier I'll add a Clock delegate to handle the dependency on the current date/time; add this to the beginning of the HomeController class:

    public Func<DateTime> Clock = () => DateTime.Now;

The GET AddPost method changes to:

    [HttpGet]
    [Authorize]
    public ActionResult AddPost()
    {
      return View(new Post { CreatedOn = Clock() });
    }

This makes all the test pass and the /Home/AddPost page shows the current date/time.

The final change I want to make is to change the Content textbox into a textarea (hard to write a whole post in that field). That can be done by changing the Post class again:

  public class Post
  {
    [HiddenInput(DisplayValue = false)]
    public Guid Id { get; set; }

    public DateTime CreatedOn { get; set; }
    public string Title { get; set; }

    [DataType(DataType.MultilineText)]
    public string Content { get; set; }
  }

Finally (!) there's one more change I want to make to the third acceptance test, a refactoring I didn't want to make until I was sure everything else was working:

    [TestMethod]
    public void LoggingInReturnsAuthenticationCookie()
    {
      const string COOKIE_NAME = ".ASPXAUTH";
      const string POST_DATA = "Username=user&Password=pass";

      var cookie = Post("/Account/LogOn", POST_DATA);

      Assert.IsTrue(cookie.StartsWith(COOKIE_NAME));
    }

The aesthetics leave a lot to be desired, as I warned you in the beginning, but at least the functionality is there. I'm happy with what I've accomplished today so I'll end this post now.

No comments: