Saturday, March 10, 2012

Write your own blog engine, part 6

Adding features

What new features can I add to this to make it more useful? Paging is one that would be immediately useful (right now I will return the most recent five posts but I have no way of getting to older ones). Comments are also necessary; I will keep them in mind for later.

There are two aspects of paging: I need to return a list of links pointing to pages and I need to be able to display that page when I click on the link. A problem comes to mind: since the posts are sorted in descending order based on their date, each time I add a new post the content of all pages will change. I don't know how this affects something like search engines; you might want to use paging based on the month of the post instead.

I'll start with the first part, the navigation links. I will return those using the new HTML5 nav tag:

<nav>
<ul>
<li><a href="/Home/Index/1">1</a></li>
<li><a href="/Home/Index/2">2</a></li>
<li><a href="/Home/Index/3">3</a></li>
</ul>
</nav>

I have decided to keep the page number even in the link to the first page, for consistency, even though /Home/Index and /Home/Index/1 should mean the same thing.

This allows me to write the acceptance test:

    [TestMethod]
    public void TheHomePageReturnsPagerStructure()
    {
      Get();

      var nav = root.SelectSingleNode("/html/body/nav");
      Assert.IsNotNull(nav);
      var pages = nav.SelectNodes("//li").ToList();
      var firstPage = pages.First();
      Assert.AreEqual("1", firstPage.InnerText);
      var firstPageLink = firstPage.SelectSingleNode("a");
      Assert.AreEqual("/Home/Index/1", firstPageLink.Attributes["href"].Value);
    }

The test fails, of course; there's no nav section in the home page. I can't jump directly to fixing the production code because I have no unit tests for the new feature. Thinking about that makes me discover a problem: right now the model for the view is a list of posts. This is a problem because I would have to add logic to the view to calculate the number of pages (dividing the list count by five), which means the view will know that each page has five posts. Not a good idea.

I will fix this by changing the model to a (new) Page class:

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

      var model = result.Model as Page;
      Assert.IsNotNull(model);
    }

Add a new Page class to the Models folder:

  public class Page
  {
  }

The code compiles and the new test fails. In order to fix it the HomeController.Index method has to change to:

    public ActionResult Index()
    {
      return View(new Page());
    }

Yes, I have deleted the existing code from the Index method; I will be forced to add it again, but it was short enough that I didn't care about losing it for now. If it were more complicated I would have probably commented it out instead.

The new unit test is green. The next thing to do is to see how many tests are now failing. Five of them are red, but three are acceptance tests; I will focus on the unit tests first. They are IndexCallsRepositoryToGetPosts and IndexPassesPostsToView; the second one should be changed to use the new Page model:

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

      var result = sut.Index() as ViewResult;

      var model = result.Model as Page;
      CollectionAssert.AreEqual(posts, model.Posts);
    }

Note the use of CollectionAssert instead of the usual Assert - that's because I don't want to verify that the model being returned is the same object, I just want to verify that it contains the same elements. That meant I also had to return some elements, otherwise the test wouldn't have been very relevant.

I need to change the Page class to make this compile:

  public class Page
  {
    public List<Post> Posts { get; set; }
  }

The same unit tests are still failing, but now I can make them pass:

    public ActionResult Index()
    {
      var posts = postRepository.GetRecentPosts().ToList();
      var page = new Page { Posts = posts };

      return View(page);
    }

Only the acceptance tests are failing now; time to see why that happens. The MSTest engine tells me the error is The remote server returned an error: (500) Internal Server Error. - trying to load the site in a browser tells me I passed the wrong model type. I will have to fix the Index view to solve that:

@{
  ViewBag.Title = "MyBlogEngine";
}
@using Page = Renfield.MyBlogEngine.MVC.Models.Page
@model Page

<h2>@ViewBag.Title</h2>

@Html.ActionLink("Create new post", "Add", "Post")

@foreach (var post in Model.Posts)
{
  <article>
    <header>
      <p><time pubdate="pubdate">@post.CreatedOn.ToLongDateString()</time></p>
      <h2>
        @Html.ActionLink(post.Title, "Details", "Post", new { id = post.Id }, null)
        <a href="@Url.Action("Edit", "Post", new { id = post.Id })"><img src="@Url.Content("~/Content/file_edit.png")" alt="edit" /></a>
        <a href="@Url.Action("Delete", "Post", new { id = post.Id })"><img src="@Url.Content("~/Content/file_delete.png")" alt="delete" /></a>
      </h2>
    </header>
    @Html.Raw(post.Content)
  </article>
}

All the tests pass again, except for the latest acceptance test, but now I have a better model for the page. I will have to add a new Count property to it since I don't want the view to have to calculate how many pages to display in the navigation section. Here's the unit test for this:

    [TestMethod]
    public void PageModelContainsCorrectCount()
    {
      var posts = new List<Post>();
      for (var i = 0; i < 7; i++)
        posts.Add(new Post());
      repository
        .Setup(it => it.GetRecentPosts())
        .Returns(posts);

      var result = sut.Index() as ViewResult;

      var model = result.Model as Page;
      Assert.AreEqual(2, model.Count);
    }

This means changing the Page class to:

  public class Page
  {
    public List<Post> Posts { get; set; }
    public int Count { get; set; }
  }

The test fails, of course. I have to add a bit of logic to the HomeController.Index method to make it pass; I don't like that, but I don't think the problem is big enough to justify introducing a new class:

    public ActionResult Index()
    {
      var posts = postRepository.GetRecentPosts().ToList();
      var page = new Page
      {
        Posts = posts,
        Count = (posts.Count - 1) / 5 + 1
      };

      return View(page);
    }

Looking at the way the page count is calculated I'm already starting to regret the decision not to move this logic to another class so that I can test it properly. I am pretty confident that the formula is correct (1 to 5 posts would return one page, 6 to 10 posts would result in two pages and so on) but it might be better to have it actually specified in code. Right now the reason behind that formula only exists in my mind. I'm still reluctant to add a new class, though, so I'll just try to watch for problems in this area.

The unit test passes and so do all the other except for the latest acceptance test. I'll change the Index view to fix that:

@{
  ViewBag.Title = "MyBlogEngine";
}
@using Page = Renfield.MyBlogEngine.MVC.Models.Page
@model Page

<h2>@ViewBag.Title</h2>

@Html.ActionLink("Create new post", "Add", "Post")

@foreach (var post in Model.Posts)
{
  <article>
    <header>
      <p><time pubdate="pubdate">@post.CreatedOn.ToLongDateString()</time></p>
      <h2>
        @Html.ActionLink(post.Title, "Details", "Post", new { id = post.Id }, null)
        <a href="@Url.Action("Edit", "Post", new { id = post.Id })"><img src="@Url.Content("~/Content/file_edit.png")" alt="edit" /></a>
        <a href="@Url.Action("Delete", "Post", new { id = post.Id })"><img src="@Url.Content("~/Content/file_delete.png")" alt="delete" /></a>
      </h2>
    </header>
    @Html.Raw(post.Content)
  </article>
}

<nav>
  <ul>
    @for (var i = 1; i <= Model.Count; i++)
    {
      <li>@Html.ActionLink(i.ToString(), "Index", new { id = i })</li>
    }
  </ul>
</nav>

This makes all the tests pass. That takes care of the first part of the paging feature; the home page returns a list of links to each page. I now have to handle the second part: clicking on a link should display the correct page. There are two possible outcomes to trying to display a given page: the page exists (and thus contains posts), or it doesn't. (I decide that trying to open an nonexistent page should redirect to the home page.)

I have a hard time writing an acceptance test, though; trying to open the /Home/Index/1 page won't help in any way because it can already be opened. (The Index method just ignores the "1" part.) Trying to open the /Home/Index/2 page won't work if there is only one page. I will solve this by simply adding six fake posts, thus guaranteeing the existence of a second page:

    [TestMethod]
    public void OpeningTheSecondPageReturnsCorrectStartingPost()
    {
      var cookie = LogOn("user", "pass");
      var guids = new List<Guid>();
      for (var i = 1; i <= 6; i++)
        guids.Add(AddPost(cookie, i));

      Get("/Home/Index/2");

      // get rid of the posts that were added
      foreach (var guid in guids)
        DeletePost(cookie, guid);
      
      Assert.AreEqual("1", title.Trim());
    }

I have added new helper methods, AddPost and DeletePost; I might use them later to refactor the other acceptance tests, but that's not going to happen while there are failing tests.

The test fails because the top title is "6" not "1" - as I said, right now the Index method ignores the page number. I will have to fix that; here's a unit test to help me do it:

    [TestMethod]
    public void IndexReturnsCorrectPostsForGivenPage()
    {
      var posts = new List<Post>();
      for (var i = 0; i < 7; i++)
        posts.Add(new Post { Title = i.ToString() });
      repository
        .Setup(it => it.GetRecentPosts())
        .Returns(posts);

      var result = sut.Index(2) as ViewResult;

      var model = result.Model as Page;
      var titles = model.Posts.Select(p => p.Title).ToList();
      CollectionAssert.AreEqual(new[] { "0", "1", "2", "3", "4" }, titles);
    }

Of course, this cannot compile because of the argument - just add an int pageNo = 1 parameter to the Index method. That will make the test compile but, of course, not pass. That requires a change in the Index method logic:

    public ActionResult Index(int pageNo = 1)
    {
      var posts = postRepository.GetPostsForPage(pageNo).ToList();
      var page = new Page
      {
        Posts = posts,
        Count = (posts.Count - 1) / 5 + 1
      };

      return View(page);
    }

The GetPostsForPage method doesn't exist; I'll add it to the interface first:

  public interface PostRepository
  {
    IEnumerable<Post> GetRecentPosts();
    IEnumerable<Post> GetPostsForPage(int pageNo);
    void Add(Post post);
    Post Get(Guid id);
    void Remove(Guid id);
    void Update(Post post);
  }

I suspect the GetRecentPosts method will no longer be needed; I will check after I make all the tests pass. First, I will add a unit test to check for a page other than the first:

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

      var result = sut.GetPostsForPage(2).ToList();

      Assert.AreEqual("4", result[0].Title);
      Assert.AreEqual("3", result[1].Title);
      Assert.AreEqual("2", result[2].Title);
      Assert.AreEqual("1", result[3].Title);
      Assert.AreEqual("0", result[4].Title);
    }

After writing a fake implementation to see the test fail I write the correct one:

    public IEnumerable<Post> GetPostsForPage(int pageNo)
    {
      var posts = blogPersistence
        .Posts
        .OrderByDescending(p => p.CreatedOn)
        .Skip((pageNo - 1) * 5)
        .Take(5)
        .ToList();

      return posts;
    }

The new unit test in the DbPostRepositoryTests class passes, but a lot of the others fail (four unit tests and an acceptance test). Looking at the failing tests tells me it's because I was setting up the GetRecentPosts call, but the new implementation of the Index method no longer uses that one. Two of them are easy to fix (all the failing unit tests are in the HomeControllerTests class):

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

      repository.Verify(it => it.GetPostsForPage(1));
    }

    [TestMethod]
    public void IndexPassesPostsToView()
    {
      var posts = new List<Post> { new Post(), new Post() };
      repository
        .Setup(it => it.GetPostsForPage(1))
        .Returns(posts);

      var result = sut.Index() as ViewResult;

      var model = result.Model as Page;
      CollectionAssert.AreEqual(posts, model.Posts);
    }

The IndexReturnsCorrectPostsForGivenPage method is no longer needed - that logic has been moved to the DbPostRepository class so I'll just delete this test. Finally, the PageModelContainsCorrectCount makes me realize there was a bug in the existing code (GetRecentPosts would only return five posts at the most, so the page count was never going to be greater than one) and this forces me to extract the page count calculation out of the controller. (It's interesting how often I find myself forced towards a good design by this style of writing code.)

Because the controllers are supposed to be extremely simple (their task is to obtain the right model and pass it on to the right view) I cannot just move the calculation to the repository class; I need a class that can return a complete Page model. I will create a PageService interface to help with that:

  public interface PageService
  {
    Page GetPage(int pageNo);
  }

This causes a cascade of changes, fortunately most of them quite small. I will start with the HomeControllerTests class:

  [TestClass]
  public class HomeControllerTests
  {
    [TestInitialize]
    public void SetUp()
    {
      service = new Mock<PageService>();
      sut = new HomeController(service.Object);
    }

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

      Assert.IsNotNull(result);
    }

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

      service.Verify(it => it.GetPage(1));
    }

    [TestMethod]
    public void IndexPassesPageToView()
    {
      var page = new Page();
      service
        .Setup(it => it.GetPage(1))
        .Returns(page);

      var result = sut.Index() as ViewResult;

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

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

      service.Verify(it => it.GetPage(2));
    }

    //

    private Mock<PageService> service;
    private HomeController sut;
  }

The HomeController class becomes:

  public class HomeController : Controller
  {
    public HomeController(PageService pageService)
    {
      this.pageService = pageService;
    }

    public ActionResult Index(int pageNo = 1)
    {
      var page = pageService.GetPage(pageNo);

      return View(page);
    }

    //

    private readonly PageService pageService;
  }

All the unit tests pass; all the acceptance tests fail (there is no implementation for the PageService interface). I will call this implementation DbPageService and start with the unit tests. First, verifying that this service calls the repository:

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

      sut.GetPage(1);

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

Since I am moving the logic to the PageService implementation it makes sense to remove that logic from the repository class; as such, I will replace both GetRecentPosts and GetPostsForPage with a single GetPosts method. Here is the modified interface:

  public interface PostRepository
  {
    IEnumerable<Post> GetPosts();
    void Add(Post post);
    Post Get(Guid id);
    void Remove(Guid id);
    void Update(Post post);
  }

In order to make the new unit test compile I will have to add a DbPageService class:

  public class DbPageService : PageService
  {
    public DbPageService(PostRepository postRepository)
    {
    }

    public Page GetPage(int pageNo)
    {
      return null;
    }
  }

I add a fake DbPostRepository.GetPosts method to make the code compile:

    public IEnumerable<Post> GetPosts()
    {
      return null;
    }

The GetPageCallsRepository unit test fails; I make it pass by the simplest method:

  public class DbPageService : PageService
  {
    public DbPageService(PostRepository postRepository)
    {
      this.postRepository = postRepository;
    }

    public Page GetPage(int pageNo)
    {
      postRepository.GetPosts();
      return null;
    }

    //

    private readonly PostRepository postRepository;
  }

Of course, this is not a very good implementation. New tests are needed to get to the correct one; I'll start by requiring that the GetPage method returns the correct posts for the first page:

    [TestMethod]
    public void GetPageReturnsCorrectPostsForFirstPage()
    {
      var posts = new List<Post>();
      for (var i = 0; i < 10; i++)
        posts.Add(new Post { Title = i.ToString(), CreatedOn = new DateTime(2000, 1, 1, 1, 1, i) });
      var repository = new Mock<PostRepository>();
      repository
        .Setup(it => it.GetPosts())
        .Returns(posts);
      var sut = new DbPageService(repository.Object);

      var result = sut.GetPage(1);

      var titles = new[] { "9", "8", "7", "6", "5" };
      CollectionAssert.AreEqual(titles, result.Posts.Select(p => p.Title).ToArray());
    }

Fixing it takes me closer to the correct implementation:

    public Page GetPage(int pageNo)
    {
      var posts = postRepository
        .GetPosts()
        .OrderByDescending(p => p.CreatedOn)
        .Skip((pageNo - 1) * 5)
        .Take(5)
        .ToList();

      return new Page { Posts = posts };
    }

This still leaves open the issue of the page count:

    [TestMethod]
    public void GetPageReturnsTheCorrectPageCount()
    {
      var posts = new List<Post>();
      for (var i = 0; i < 10; i++)
        posts.Add(new Post { Title = i.ToString(), CreatedOn = new DateTime(2000, 1, 1, 1, 1, i) });
      var repository = new Mock<PostRepository>();
      repository
        .Setup(it => it.GetPosts())
        .Returns(posts);
      var sut = new DbPageService(repository.Object);

      var result = sut.GetPage(1);

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

This change makes the new test pass:

    public Page GetPage(int pageNo)
    {
      var posts = postRepository
        .GetPosts();

      var pagePosts = posts
        .OrderByDescending(p => p.CreatedOn)
        .Skip((pageNo - 1) * 5)
        .Take(5)
        .ToList();
      var pageCount = (posts.Count() - 1) / 5 + 1;

      return new Page { Posts = pagePosts, Count = pageCount };
    }

ReSharper warns me that the posts IEnumerable is enumerated more than once; in this case I will ultimately make two queries to the database. I don't see how I can solve that problem without overly complicating the code. (If there's an easy solution, please let me know.)

Since all the tests in the DbPageServiceTests class pass I can take the time for a quick refactoring:

  [TestClass]
  public class DbPageServiceTests
  {
    [TestInitialize]
    public void SetUp()
    {
      repository = new Mock<PostRepository>();
      sut = new DbPageService(repository.Object);
    }

    [TestMethod]
    public void GetPageCallsRepository()
    {
      sut.GetPage(1);

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

    [TestMethod]
    public void GetPageReturnsCorrectPostsForFirstPage()
    {
      CreatePosts(10);

      var result = sut.GetPage(1);

      var titles = new[] { "9", "8", "7", "6", "5" };
      CollectionAssert.AreEqual(titles, result.Posts.Select(p => p.Title).ToArray());
    }

    [TestMethod]
    public void GetPageReturnsTheCorrectPageCount()
    {
      CreatePosts(10);

      var result = sut.GetPage(1);

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

    //

    private Mock<PostRepository> repository;
    private DbPageService sut;

    private void CreatePosts(int upper)
    {
      var posts = new List<Post>();
      for (var i = 0; i < upper; i++)
        posts.Add(new Post { Title = i.ToString(), CreatedOn = new DateTime(2000, 1, 1, 1, 1, i) });
      repository
        .Setup(it => it.GetPosts())
        .Returns(posts);
    }
  }

The tests are still passing. So do all the other unit tests, in fact. I should try to fix the acceptance tests too; I will do that by registering the new implementation in the MunqMvc3Startup.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());
      ioc.Register<PageService>(c => new DbPageService(c.Resolve<PostRepository>()));
    }

This doesn't yet fix the tests, though; when I try to open the site in a browser I am told that "Value cannot be null." Looking at the code for a bit shows me that the culprit is the DbPostRepository.GetPosts method:

    public IEnumerable<Post> GetPosts()
    {
      return null;
    }

Not a big deal. I will start by deleting the GetRecentPosts and GetPostsForPage methods from the DbPostRepository class and all the unit tests that reference those two from the DbPostRepositoryTests class (the first four unit tests). I will then add a new unit test:

    [TestMethod]
    public void GetPostsReturnsAllRecordsFromThePostsTable()
    {
      postsTable.Add(new Post { Title = "1" });
      postsTable.Add(new Post { Title = "2" });

      var result = sut.GetPosts();

      CollectionAssert.AreEqual(new[] { "1", "2" }, result.Select(p => p.Title).ToArray());
    }

Making this pass is very easy:

    public IEnumerable<Post> GetPosts()
    {
      return blogPersistence.Posts;
    }

Everything passes now except for the most recent acceptance test. This tells me that there's a problem with retrieving the pages (and, indeed, if I test the application in a browser I can see that). I check the DbPageServiceTests class and find out that I only test the first page. I will write another unit test to verify the second page:

    [TestMethod]
    public void GetPageReturnsCorrectPostsForSecondPage()
    {
      CreatePosts(10);

      var result = sut.GetPage(2);

      var titles = new[] { "4", "3", "2", "1", "0" };
      CollectionAssert.AreEqual(titles, result.Posts.Select(p => p.Title).ToArray());
    }

Hmm… this passes. That means the problem must be in the HomeController… but the Index method is as simple as can be:

    public ActionResult Index(int pageNo = 1)
    {
      var page = pageService.GetPage(pageNo);

      return View(page);
    }

More importantly, I have a test verifying that the Index method requests the correct page from the service:

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

      service.Verify(it => it.GetPage(2));
    }

It takes me a bit before I figure it out: the MVC engine uses id as the default name for the page parameter. I change the Index method accordingly:

    public ActionResult Index(int id = 1)
    {
      var page = pageService.GetPage(id);

      return View(page);
    }

Whew. Everything works, including in the browser. I got scared there for a bit. To make sure everything is ok I re-enable the two disabled acceptance tests; they work too. The pager works too (in the browser), though it looks extremely ugly; I will have to do something about that.

In the meantime, however, I discover something by accident: I deleted all records from the Posts table and I'm not seeing the default post that was supposed to be there in this case. Three acceptance tests are failing (I have disabled the AddingAPostReturnsItOnTheHomePage and DisplayingAParticularPost tests again). It seems that the DbPostRepositoryTests class needs a new unit test:

    [TestMethod]
    public void GetPostsReturnsFakePostIfTableIsEmpty()
    {
      var result = sut.GetPosts().ToList();

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

The GetPosts method needs a small change to make this pass:

    public IEnumerable<Post> GetPosts()
    {
      var posts = blogPersistence.Posts;

      return posts.Any()
               ? posts.AsEnumerable()
               : new List<Post> { GetFakePost() };
    }

This makes all the tests pass, including the two that are normally disabled. Whew, this was a bit tense a few times.

The only refactoring opportunities I see for now is replacing a couple of

      Post("/Post/Delete/" + guid, "", cookie);

calls with

      DeletePost(cookie, guid);

but everything looks ok otherwise. (Of course, all the tests are still green.) I guess the only thing remaining is to fix the ugly page navigation section. Add this at the end of the Site.css file to fix that:

nav ul
{
    list-style: none;
    margin: 0;
    padding: 0;
    overflow: hidden;
}

nav li
{
    float: left;
}

nav a:link, nav a:visited
{
    display: block;
    width: 30px;
    background-color: #98bf21;
    text-align: center;
    padding: 4px;
    text-decoration: none;
}

nav a:hover, nav a:active
{
    background-color: #7a991a;
}

This makes the pager look much better. (Thanks to http://www.w3schools.com/css/css_navbar.asp and http://www.w3schools.com/css/tryit.asp?filename=trycss_navbar_horizontal_float_advanced for showing me how to do this.)

Next time I will start working on comments.

Sunday, March 04, 2012

Smoking causes cancer

Most people are idiots. You can easily prove this because they will agree with the claim in the title. If you try to give them a counterexample — like, you know, the vast majority of smokers not having cancer — they will counter it by "well, it doesn't cause cancer right away". Causality doesn't work like that; it's a "if X, then Y" relationship. Don't use the word if you mean something other than that; say "smoking increases the probability of getting cancer" (yes it does).

As an additional counterexample to the claim, here's something most people will disagree with: passing under a ladder brings bad luck. Mark Twain had a humorous take on this: a character tells another "X passed under a ladder and only six months later he fell and broke his arm" (I'm quoting from memory). That's exactly the same type of "causality" as in the case of smoking: you do X, then long after that bad thing Y happens, and you claim Y is caused by X. This is pure superstition; most people know that in the case of the ladder, but not in the case of smoking.

As for why: as usual, it's because of religion. Most people, as I said, are idiots and they prefer not to think; being told what to think is easier for them. Since the high priests of the current reigning religion of the world (aka science) tell them that walking under ladders does not cause bad luck, they accept it; when the priests tell them that smoking does cause cancer, they accept that too.

As I have mentioned before, I have lost my faith in education as a weapon against this kind of stupidity. People today are infinitely more educated than those living a hundred years ago and they still have the same lack of desire to challenge their own opinions (Paul Graham has a great article about this). This conflicts with my powerful belief that humanity always improves socially long-term (we're better now than a hundred years ago and so on), so that's a contradiction I haven't solved yet.

Oh well; end of rant. As the non-A adepts say, be precise; if you believe that smoking is bad because it negatively affects one's health, say that; saying "smoking kills" is more emotional but also incredibly stupid.

Thursday, March 01, 2012

World history History of the human race

Here's the official version:
  • In the first ten thousand years, people didn't do much of anything.
  • In the second ten thousand years, people didn't do much of anything.
  • In the third ten thousand years, people didn't do much of anything.
  • In the fourth ten thousand years, people didn't do much of anything.
  • ... repeat for several million years ...
  • Finally, in the last ten thousand years people have started spreading out and inventing cars, airplanes and other interesting stuff.

Write your own blog engine, part 5

Post management

I have decided to change the scope of this chapter: it won't be limited just to deleting but instead it would be about general post management: identifying, adding, deleting and modifying posts.

I will use the value of the Id field to identify posts; that means that an URL like /Home/Post/guid would take me to a specific post. However, before I start to go in that direction I realize that it makes more sense to create a new PostController class to handle everything about posts. That way, the URL for adding them would be /Post/Add and the one for displaying a single post would be /Post/Details/guid.

That means changing a lot of tests; I verify that they're all green, including the one that I disabled earlier (they are) and then start searching for the AddPost string in the tests. The first one is in the acceptance test I just re-enabled so I'll change "Home/AddPost" to "Post/Add". It is now failing with a (404) Not Found. error.

The next occurence of the AddPost string is in the HomeControllerTests class. I create a new PostControllerTests class and move the tests over, changing them as needed:

  [TestClass]
  public class PostControllerTests
  {
    [TestInitialize]
    public void SetUp()
    {
      repository = new Mock<PostRepository>();
      sut = new PostController(repository.Object);
    }

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

      sut.Add(post);

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

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

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

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

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

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

      var result = sut.Add() as ViewResult;

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

    //

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

Note that the POST_AddPostRedirectsToTheHomePage test needs to check the controller name too, not just the action name.

I will now add a new PostController class to the MVC project and move the actions over from HomeController:

  public class PostController : Controller
  {
    public Func<DateTime> Clock = () => DateTime.Now;

    public PostController(PostRepository postRepository)
    {
      this.postRepository = postRepository;
    }

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

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

      return RedirectToAction("Index", "Home");
    }

    //

    private readonly PostRepository postRepository;
  }

Of course, the Views/Home/AddPost.cshtml view will have to become Views/Post/Add.cshtml; I make that change and re-run the tests. They all run successfully.

I run the application (Ctrl-F5); everything seems ok (I do have two test posts but that's fine). The "Create new post" link is wrong though, still pointing to the old action, so I change the action link in Index.cshtml to:

@Html.ActionLink("Create new post", "Add", "Post")

I disable the last acceptance test and re-run everything; it's all green. A good start.

The next feature I'm going to work on is displaying an individual post. I want to be able to go to /Post/Details/guid and see a single post. Here's the acceptance test for that:

    [TestMethod]
    public void DisplayingAParticularPost()
    {
      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("/Post/Add", POST_DATA, cookie);
      Get("/Post/Details/" + guid);

      var article = root.SelectSingleNode("/html/body/article");
      var header = article.SelectSingleNode("header");
      var title = header.SelectSingleNode("h2");
      Assert.AreEqual(guid.ToString(), title.InnerText);
    }

Since I don't have a post with a known GUID I have to add one first. I know that works because I tested it before.

The unit test for this feature is rather simple; remember that it has to be added to the new PostControllerTests class:

    [TestMethod]
    public void GET_DetailsReturnsExistingPost()
    {
      var guid = Guid.NewGuid();
      var post = new Post();
      repository
        .Setup(it => it.Get(guid))
        .Returns(post);

      var result = sut.Details(guid) as ViewResult;

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

This means adding a new method to the PostRepository interface:

    Post Get(Guid id);

and the matching implementation in the DbPostRepository class:

    public Post Get(Guid id)
    {
      return blogPersistence
        .Posts
        .Where(it => it.Id == id)
        .FirstOrDefault();
    }

I added production code without a matching test. I hope the logic is simple enough for this not to be a problem… I hope I won't regret this later.

Anyway, back to the test. I need to add the Details method to the PostController class; first, just enough to get it to compile:

    public ActionResult Details(Guid id)
    {
      return null;
    }

Then, after I verified that the test fails, the correct implementation:

    public ActionResult Details(Guid id)
    {
      return View(postRepository.Get(id));
    }

The test is now passing. Ok, next issue: what should I return for an inexistent guid? (Someone might have an URL to a post I deleted, for example.) I think about it for a while and decide that this request should return a 404 Not Found error. Here's the unit test for it:

    [TestMethod]
    public void GET_DetailsReturns404ForNonExistingPost()
    {
      var result = sut.Details(Guid.NewGuid()) as HttpStatusCodeResult;

      Assert.AreEqual(404, result.StatusCode);
    }

The test fails (good) so I change the production code to make it pass:

    public ActionResult Details(Guid id)
    {
      var post = postRepository.Get(id);
      if (post == null)
        return new HttpStatusCodeResult(404);

      return View(post);
    }

The tests pass now except for the new acceptance test. I'll add the new Details.cshtml view to solve that problem:

@{
  ViewBag.Title = "Post details";
}

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

<h2>@ViewBag.Title</h2>

@Html.DisplayForModel()

Hmm… it doesn't actually solve the problem and, in fact, when I try to go to a /Post/Details/guid URL (with a guid that I know exists because it's being displayed on the home page), I get back a 404. I check the database and discover that the Id and Title columns don't match in the Posts table; I forgot to send the Id and it was automatically generated and thus didn't match the title.

I change the acceptance test to send the Id and retest; it's still crashing, this time for some other reason: it doesn't find the article header. I forgot about this when I wrote the view; here's the modified view:

@{
  ViewBag.Title = "Post details";
}

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

<h2>@ViewBag.Title</h2>

<article>
  <header>
    <p><time pubdate="pubdate">@Model.CreatedOn.ToLongDateString()</time></p>
    <h2>@Model.Title</h2>
  </header>
  @Html.Raw(Model.Content)
</article>

All the tests are passing now - success! All that remains is changing the home page view so that the post titles are links to the corresponding /Post/Details action; change the ActionLink line to:

      <h2>@Html.ActionLink(post.Title, "Details", "Post", new { id = post.Id }, null)</h2>

The home page view looks much better now (in a manner of speaking).

What about the delete function? I've decided that I'm going to place two small icons to the right of each title, one for edit and one for delete. I found the two icons using the highly recommended iconfinder site; search for "iconset:vaga". (You don't have to use the same icons, of course, but these are free for commercial use.) I added the two .png files to the Content folder.

I want to see how this would look so I start by changing the Index.cshtml view:

@{
  ViewBag.Title = "MyBlogEngine";
}
@using Renfield.MyBlogEngine.MVC.Models
@model IEnumerable<Post>

<h2>@ViewBag.Title</h2>

@Html.ActionLink("Create new post", "Add", "Post")

@foreach (var post in Model)
{
  <article>
    <header>
      <p><time pubdate="pubdate">@post.CreatedOn.ToLongDateString()</time></p>
      <h2>
        @Html.ActionLink(post.Title, "Details", "Post", new { id = post.Id }, null)
        <a href="@Url.Action("Edit", "Post", new { id = post.Id })"><img src="@Url.Content("~/Content/file_edit.png")" alt="edit" /></a>
        <a href="@Url.Action("Delete", "Post", new { id = post.Id })"><img src="@Url.Content("~/Content/file_delete.png")" alt="delete" /></a>
      </h2>
    </header>
    @Html.Raw(post.Content)
  </article>
}

It looks good and the tests are still passing. (Hmm… I forgot to disable the last acceptance test and I got a new test post. Add the [Ignore] attribute to it before we consume all the GUIDs in existence. Yes, that's a feeble joke, har har.)

In fact, now that I looked at the acceptance tests I realize that I forgot to remove the duplication. Since all the tests pass, I can do it now and it's better not to accumulate more technical debt before taking care of it. Here's the refactored class:

  [TestClass]
  public class AcceptanceTests
  {
    [TestMethod]
    public void HomePageHasCorrectTitle()
    {
      Get();

      var pageTitle = root.SelectSingleNode("/html/head/title").InnerText;
      Assert.AreEqual("MyBlogEngine", pageTitle);
    }

    [TestMethod]
    public void HomePageReturnsMostRecentArticles()
    {
      Get();

      Assert.IsNotNull(header);
      Assert.IsNotNull(title);
      var date = header.SelectSingleNode("//time[@pubdate]");
      Assert.IsNotNull(date);
    }

    [TestMethod]
    public void LoggingInReturnsAuthenticationCookie()
    {
      const string COOKIE_NAME = ".ASPXAUTH";

      var cookie = LogOn("user", "pass");

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

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

      Post("/Post/Add", POST_DATA, cookie);

      Get();
      Assert.AreEqual(guid.ToString(), title.Trim());
    }

    //[Ignore]
    [TestMethod]
    public void DisplayingAParticularPost()
    {
      var guid = Guid.NewGuid();
      var POST_DATA = string.Format("Id={0}&Title={0}&Content=This is a test", guid);
      var cookie = LogOn("user", "pass");
      Post("/Post/Add", POST_DATA, cookie);

      Get("/Post/Details/" + guid);

      Assert.AreEqual(guid.ToString(), title);
    }

    //

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

    private void Get(string relativeUrl = "")
    {
      using (var web = new WebClient())
      {
        var html = web.DownloadString(BASE_URL + relativeUrl);

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

        var articles = root.SelectNodes("/html/body/article").ToList();
        var topArticle = articles.First();
        header = topArticle.SelectSingleNode("header");
        var h2 = header.SelectSingleNode("h2");
        title = h2 == null ? null : h2.InnerText;
      }
    }

    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"];
      }
    }

    private static string LogOn(string username, string password)
    {
      return Post("/Account/LogOn", string.Format("Username={0}&Password={1}", username, password));
    }
  }

Note that I've had to change the test in the AddingAPostReturnsItOnTheHomePage method because the title being discovered has some extra newlines and spaces, due to the additional stuff I put in the h2 tag. Also, don't forget to uncomment the [Ignore] attributes after running the tests.

Back to the features (my puns are horrible). I am going to start with deleting a post so I can get rid of all the test posts I've added. Of course, to make sure that there is something to delete I will have to add it first. I'll start with the acceptance test:

    [TestMethod]
    public void AddingThenDeletingAPostMakesItDisappear()
    {
      var guid = Guid.NewGuid();
      var POST_DATA = string.Format("Id={0}&Title={0}&Content=This is a test", guid);
      var cookie = LogOn("user", "pass");
      Post("/Post/Add", POST_DATA, cookie);

      Post("/Post/Delete/" + guid, "", cookie);

      try
      {
        Get("/Post/Details/" + guid);
      }
      catch (WebException ex)
      {
        if (ex.Message.Contains("(404)"))
          return;
      }

      Assert.Fail("/Post/Details did not return 404");
    }

As you can see, I am adding then deleting the post and then I'm checking for its existence. If trying to access its details returns 404 that means the delete worked. Otherwise, either there was another error or the /Post/Details access succeeded (so the delete failed).

One problem with the above test, though, is that I am going to need both GET and POST methods for the Delete action and both have the same signature (they take a single argument, the guid of the post to delete). Fortunately I found this article that indicates a solution: change the name of the POST method but use the [ActionName] attribute to keep the URL with the same signature as the GET method. Like in that example, I am going to use the name DeleteConfirmed for the POST method.

Here is the unit test for deleting an existing post:

    [TestMethod]
    public void POST_DeleteRemovesFromRepository()
    {
      var guid = Guid.NewGuid();
      var post = new Post();
      repository
        .Setup(it => it.Get(guid))
        .Returns(post);

      sut.DeleteConfirmed(guid);

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

This requires a new method in the PostRepository class:

    void Remove(Post post);

The empty implementation of the DeleteConfirmed method is:

    [HttpPost, ActionName("Delete")]
    [Authorize]
    public ActionResult DeleteConfirmed(Guid id)
    {
      return null;
    }

I also need an implementation of the Remove method in the DbPostRepository class. This time I'll start with a test in the DbPostRepositoryTests class:

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

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

The test fails with an empty implementation so I write the correct one:

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

Back to the PostController class, I'll replace the empty implementation (after confirming that the test fails) with the correct one:

    [HttpPost, ActionName("Delete")]
    [Authorize]
    public ActionResult DeleteConfirmed(Guid id)
    {
      var post = postRepository.Get(id);
      postRepository.Remove(post);

      return null;
    }

This time the test passes. Of course, the method is not fully specified: I need to decide what happens if the post is not found and what should be returned on success. I decide that nothing should happen in the first case and that the method should redirect to the home page when it ends. That means two unit tests, one for an existing post and one for a non-existent post, both verifying that the method is redirecting to the home page. Here they are:

    [TestMethod]
    public void POST_DeleteRedirectsToTheHomePageForExistentPost()
    {
      var guid = Guid.NewGuid();
      repository
        .Setup(it => it.Get(guid))
        .Returns(new Post());

      var result = sut.DeleteConfirmed(guid) as RedirectToRouteResult;

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

    [TestMethod]
    public void POST_DeleteRedirectsToTheHomePageForNonExistentPost()
    {
      var guid = Guid.NewGuid();

      var result = sut.DeleteConfirmed(guid) as RedirectToRouteResult;

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

A simple change to the DeleteConfirmed method makes them both pass:

    [HttpPost, ActionName("Delete")]
    [Authorize]
    public ActionResult DeleteConfirmed(Guid id)
    {
      var post = postRepository.Get(id);
      postRepository.Remove(post);

      return RedirectToAction("Index", "Home");
    }

I don't like the fact that I'm still calling the Remove method of the repository even if the Get method returned null. It would be better if that part looked like this:

      var post = postRepository.Get(id);
      if (post != null)
        postRepository.Remove(post);

However, I really don't like the "feature envy" expressed by this method: I call a method on the repository and then, depending on its result, call another method, also on the repository. This is dumb: since all the knowledge needed is in the repository, this whole thing should be moved there. The controller should only call the Remove method and expect it to do its job.

This means I have to go back for a bit and start over. Change the signature of the PostRepository.Remove method to:

    void Remove(Guid id);

Change the implementation to an empty method:

    public void Remove(Guid id)
    {
    }

Delete the last unit test from the PostControllerTests class and change the remaining two to:

    [TestMethod]
    public void POST_DeleteRemovesFromRepository()
    {
      var guid = Guid.NewGuid();

      sut.DeleteConfirmed(guid);

      repository.Verify(it => it.Remove(guid));
    }

    [TestMethod]
    public void POST_DeleteRedirectsToTheHomePage()
    {
      var result = sut.DeleteConfirmed(Guid.NewGuid()) as RedirectToRouteResult;

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

Finally, change the DbPostRepositoryTests unit test to:

    [TestMethod]
    public void RemoveDeletesThePostFromTheDatabase()
    {
      var guid = Guid.NewGuid();
      var post = new Post { Id = guid };
      posts.Add(post);
      SetUpPosts();

      sut.Remove(guid);

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

The test fails because the implementation does nothing. Easy to fix:

    public void Remove(Guid id)
    {
      var post = Get(id);

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

The test crashes and burns. It seems that not having tests for the Get method did come back to haunt me. Ouch.

It seems I can't get away with just the small amount of mocking I've done for IDbSet<T> and need the full thing. Here it is (thanks to this article); add this class to the MyBlogEngine.Tests project:

  public class FakeDbSet<T> : IDbSet<T>
    where T : class
  {
    public readonly ObservableCollection<T> _data;
    public readonly IQueryable _query;

    public FakeDbSet()
    {
      _data = new ObservableCollection<T>();
      _query = _data.AsQueryable();
    }

    public virtual T Find(params object[] keyValues)
    {
      throw new NotImplementedException("Derive from FakeDbSet<T> and override Find");
    }

    public T Add(T item)
    {
      _data.Add(item);
      return item;
    }

    public T Remove(T item)
    {
      _data.Remove(item);
      return item;
    }

    public T Attach(T item)
    {
      _data.Add(item);
      return item;
    }

    public T Detach(T item)
    {
      _data.Remove(item);
      return item;
    }

    public T Create()
    {
      return Activator.CreateInstance<T>();
    }

    public TDerivedEntity Create<TDerivedEntity>() where TDerivedEntity : class, T
    {
      return Activator.CreateInstance<TDerivedEntity>();
    }

    public ObservableCollection<T> Local
    {
      get { return _data; }
    }

    Type IQueryable.ElementType
    {
      get { return _query.ElementType; }
    }

    Expression IQueryable.Expression
    {
      get { return _query.Expression; }
    }

    IQueryProvider IQueryable.Provider
    {
      get { return _query.Provider; }
    }

    IEnumerator IEnumerable.GetEnumerator()
    {
      return _data.GetEnumerator();
    }

    IEnumerator<T> IEnumerable<T>.GetEnumerator()
    {
      return _data.GetEnumerator();
    }
  }

This means changing the DbPostRepositoryTests class; here is the new version:

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

    [TestMethod]
    public void GetRecentPostsReturnsFakePostIfTableIsEmpty()
    {
      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++)
        postsTable.Add(new Post());

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

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

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

      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);
    }

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

      sut.AddPost(post);

      Assert.IsTrue(postsTable._data.Contains(post));
      persistence.Verify(it => it.SaveChanges());
    }

    [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);
    }

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

      sut.AddPost(post);

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

    [TestMethod]
    public void RemoveDeletesThePostFromTheDatabase()
    {
      var guid = Guid.NewGuid();
      var post = new Post { Id = guid };
      postsTable.Add(post);

      sut.Remove(guid);

      Assert.IsFalse(postsTable._data.Contains(post));
      persistence.Verify(it => it.SaveChanges());
    }

    //

    private FakeDbSet<Post> postsTable;
    private Mock<BlogPersistence> persistence;
    private DbPostRepository sut;
  }

All the tests pass (yes, including the latest acceptance test). However, I am not satisfied with the DbPostRepository.Remove method - it would happily call Remove on a null result from the Get call. I need to make sure that doesn't happen with a new unit test:

    [TestMethod]
    public void RemoveReturnsImmediatelyIfThePostIsNotFound()
    {
      sut.Remove(Guid.NewGuid());

      persistence.Verify(it => it.SaveChanges(), Times.Never());
    }

I don't have any easy way to verify that Remove wasn't called but I can at least do that for SaveChanges.

The test fails so I change the production code to:

    public void Remove(Guid id)
    {
      var post = Get(id);
      if (post == null)
        return;

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

All the tests pass again. That is always a good time to look for refactoring possibilities; I can only find something minor: I'll rename the PostRepository.AddPost method to just Add.

I still can't use the application to get rid of all those test posts, though, because there's no GET Delete action; I'll start fixing that by writing a test:

    [TestMethod]
    public void GET_DeleteReturnsView()
    {
      var result = sut.Delete(Guid.NewGuid()) as ViewResult;

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

This means I need a new method in the controller. I verify that the empty return null; implementation makes the test fail and then write the correct one:

    [HttpGet]
    [Authorize]
    public ActionResult Delete(Guid id)
    {
      return View();
    }

This means adding the corresponding view:

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

@model object

<h2>@ViewBag.Title</h2>

@using (Html.BeginForm())
{
  <p>Are you sure you want to delete this post? Press CONFIRM to delete it or the Back button to return to the previous page.</p>
  <input type="submit" value="CONFIRM" />
}

It works as designed but I think it could be improved: the Delete view should display the post. This means changing the test to verify that the model is being returned:

    [TestMethod]
    public void GET_DeleteReturnsView()
    {
      var guid = Guid.NewGuid();
      var post = new Post();
      repository
        .Setup(it => it.Get(guid))
        .Returns(post);

      var result = sut.Delete(guid) as ViewResult;

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

This means changing the controller method to:

    [HttpGet]
    [Authorize]
    public ActionResult Delete(Guid id)
    {
      var post = postRepository.Get(id);

      return View(post);
    }

and the view to:

@{
  ViewBag.Title = "Delete post";
}
@using Renfield.MyBlogEngine.MVC.Models
@model Post

<h2>@ViewBag.Title</h2>

@using (Html.BeginForm())
{
  <p>Are you sure you want to delete this post? Press CONFIRM to delete it or the Back button to return to the previous page.</p>
  <input type="submit" value="CONFIRM" />
  
  <article>
    <header>
      <p><time pubdate="pubdate">@Model.CreatedOn.ToLongDateString()</time></p>
      <h2>@Model.Title</h2>
    </header>
    @Html.Raw(Model.Content)
  </article>
}

All the tests are still passing. I'm still not done with the GET Delete method, though: I need to figure out what to do if the post doesn't exist. (I could have the same blog open in two tabs or two windows; what should happen if I try to delete a post that I've already deleted?) I decide that redirecting to the home page is the correct response so I add a unit test for that:

    [TestMethod]
    public void GET_DeleteRedirectsToHomePageIfPostDoesNotExist()
    {
      var result = sut.Delete(Guid.NewGuid()) as RedirectToRouteResult;

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

Fixing the test means changing the Delete method:

    [HttpGet]
    [Authorize]
    public ActionResult Delete(Guid id)
    {
      var post = postRepository.Get(id);
      if (post == null)
        return RedirectToAction("Index", "Home");

      return View(post);
    }

All the tests pass. (I temporarily re-enabled the two disabled acceptance tests to make sure.)

The last feature in the "post management" category is editing - changing a post. I'll start with an acceptance test:

    [TestMethod]
    public void EditingAPostChangesItsProperties()
    {
      var guid = Guid.NewGuid();
      var POST_DATA1 = string.Format("Id={0}&Title=abc&Content=This is a test", guid);
      var cookie = LogOn("user", "pass");
      Post("/Post/Add", POST_DATA1, cookie);
      var POST_DATA2 = string.Format("Id={0}&Title=def&Content=This is a modified test", guid);

      Post("/Post/Edit", POST_DATA2, cookie);

      Get("/Post/Details/" + guid);
      Assert.AreEqual("def", title);
      Assert.AreEqual("This is a modified test", content);
      Post("/Post/Delete/" + guid, "", cookie);
    }

If everything is ok I delete the post so as not to clutter the blog. If the test fails, though, the test will stop and I can see the post (and hopefully figure out what's wrong).

For this to work I had to add a new declaration:

    private string content;

and add this at the end of the Get method, before the end of the using:

        content = topArticle.InnerText;

The test fails because there's no /Edit path. Here's the unit test for the default case, when there's a post with the given GUID:

    [TestMethod]
    public void POST_EditModifiesExistingPost()
    {
      var guid = Guid.NewGuid();
      var post = new Post { Id = guid };

      sut.Edit(post);

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

Just like with the Delete method, I am going to delegate the entire logic to the repository class.

This means adding a new method to the PostRepository interface:

    void Update(Post post);

and an empty (for now) implementation of the POST Edit action:

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

Note that there is no conflict here between the GET and POST methods, like it was with Delete; that's because the GET method takes the GUID as a parameter, while the POST needs the whole object.

I still need to add an empty implementation for the Update method in the DbPostRepository class so that I can compile:

    public void Update(Post post)
    {
    }

The unit test fails so I replace the empty implementation with one that's a bit better:

    [HttpPost]
    [Authorize]
    public ActionResult Edit(Post post)
    {
      postRepository.Update(post);

      return null;
    }

That unit test passes now. What should I do after I update the post? I think re-displaying the post is the best thing:

    [TestMethod]
    public void POST_EditRedirectsToDetails()
    {
      var guid = Guid.NewGuid();
      var post = new Post { Id = guid };

      var result = sut.Edit(post) as RedirectToRouteResult;

      Assert.AreEqual("Details", result.RouteValues["action"]);
      Assert.IsNull(result.RouteValues["controller"]);
      Assert.AreEqual(guid, result.RouteValues["id"]);
    }

Fixing this requires a minor change:

    [HttpPost]
    [Authorize]
    public ActionResult Edit(Post post)
    {
      postRepository.Update(post);

      return RedirectToAction("Details", new { id = post.Id });
    }

The acceptance test is still not passing because the implementation of the Update method in the repository is empty. I'll add a unit test for the "everything is ok" case:

    [TestMethod]
    public void UpdateReplacesThePostInTheDatabase()
    {
      var guid = Guid.NewGuid();
      var post = new Post { Id = guid, Title = "a", Content = "aa" };
      postsTable.Add(post);
      var newPost = new Post { Id = guid, Title = "b", Content = "bb" };

      sut.Update(newPost);

      var dbPost = postsTable._data.Where(it => it.Id == guid).First();
      Assert.AreEqual("b", dbPost.Title);
      Assert.AreEqual("bb", dbPost.Content);
    }

Fixing it means writing the correct implementation:

    public void Update(Post post)
    {
      var existingPost = Get(post.Id);

      existingPost.Title = post.Title;
      existingPost.Content = post.Content;

      blogPersistence.SaveChanges();
    }

Finally, I need a test to cover the "post not found" case:

    [TestMethod]
    public void UpdateReturnsImmediatelyIfThePostIsNotFound()
    {
      sut.Update(new Post { Id = Guid.NewGuid() });

      persistence.Verify(it => it.SaveChanges(), Times.Never());
    }

The fix is:

    public void Update(Post post)
    {
      var existingPost = Get(post.Id);
      if (existingPost == null)
        return;

      existingPost.Title = post.Title;
      existingPost.Content = post.Content;

      blogPersistence.SaveChanges();
    }

All the tests pass now except for the acceptance test. That one fails because the InnerText property returns more than I wanted (it also returns the text inside the subtags). It's easier to fix it by changing the acceptance text:

    [TestMethod]
    public void EditingAPostChangesItsProperties()
    {
      var guid = Guid.NewGuid();
      var POST_DATA1 = string.Format("Id={0}&Title=abc&Content=This is a test", guid);
      var cookie = LogOn("user", "pass");
      Post("/Post/Add", POST_DATA1, cookie);
      var POST_DATA2 = string.Format("Id={0}&Title=def&Content=This is a modified test", guid);

      Post("/Post/Edit", POST_DATA2, cookie);

      Get("/Post/Details/" + guid);
      Assert.AreEqual("def", title);
      Assert.IsTrue(content.Contains("This is a modified test"));
      Post("/Post/Delete/" + guid, "", cookie);
    }

I'm not done yet: I neeed the GET version of the Edit action:

    [TestMethod]
    public void GET_EditReturnsExistingPost()
    {
      var guid = Guid.NewGuid();
      var post = new Post();
      repository
        .Setup(it => it.Get(guid))
        .Returns(post);

      var result = sut.Edit(guid) as ViewResult;

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

After making it compile (and fail) with an empty implementation, here is the correct one:

    [HttpGet]
    [Authorize]
    public ActionResult Edit(Guid id)
    {
      var post = postRepository.Get(id);

      return View(post);
    }

This needs a view:

@{
  ViewBag.Title = "Edit a post";
}

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

<h2>@ViewBag.Title</h2>

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

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

Finally (!), I need to decide what should happen if the post I try to edit doesn't exist (in the GET method I mean). I think the best response is to return 404 like in the Details method:

    [TestMethod]
    public void GET_EditReturns404ForNonExistingPost()
    {
      var result = sut.Edit(Guid.NewGuid()) as HttpStatusCodeResult;

      Assert.AreEqual(404, result.StatusCode);
    }

It fails (I know this is boring, but it's important to always verify) so I fix the code:

    [HttpGet]
    [Authorize]
    public ActionResult Edit(Guid id)
    {
      var post = postRepository.Get(id);
      if (post == null)
        return new HttpStatusCodeResult(404);

      return View(post);
    }

All tests are green. Whew… we added quite a lot of feature.

Next time I will look into adding new features, starting with comments.