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.
Comments