Refactoring the ASP.NET MVC project template authentication – Part 2
When we left off last time we had cleaned up the Login action method. Now lets look at the Logout method:
1: public ActionResult Logout() {
2: FormsAuth.SignOut();
3:
4: return RedirectToAction("Index", "Home");
5: }
Looks pretty innocent really. But there are two problems, one for each line. First the FormsAuth.SignOut() prevents us from unit testing.
I can here the non-testing folks out there screaming that its a waste to test this method. And as it stands that maybe true. But tomorrow your boss is going to walk in and tell you that the client has asked that an activity report get emailed when a user logs out. So you need to create a unit test that makes sure that this method calls the needed code to send the message. For your test you are going to mock or stub all those dependencies. You just need to test that they are called not that they perform those actions. Testing that those dependencies do what they need to do is the job of the guy writing those services. Why do you need to make sure they are called? Because you need to make sure that when your fellow programmer two cubes over decides to remove those calls for some reason, the continuous integration build/tests will fail.
The second line creates a redirect ActionResult that sends us to the Index action on the HomeController. What happens when the client asked to send the user to a “thanks for visiting” page. We have to recompile code??? if that’s not a code smell I don’t know what is.
So what’s the solution to these problems, once again we’ll look to a custom ActionResult.
1: public class FormsLogoutResult : ActionResult {
2: public FormsLogoutResult() {}
3:
4: public override void ExecuteResult(ControllerContext context) {
5: FormsAuthentication.SignOut();
6: context.HttpContext.Response.Redirect(FormsAuthentication.DefaultUrl);
7: }
8: }
The asp.net framework has provided us with the DefaultUrl which can be set as an attribute on the forms tag in the web.config. No sense reinventing the wheel here unless the requirements change. And if they do we are changing only one class.
So here’s our final method refactored and ready for testing:
1: public ActionResult Logout() {
2: return new FormsLogoutResult();
3: }
Once again these are basic examples and there’s certainly room for improvement. If you have a suggestion please let me know.
Be sure to check out the rest of this series of posts: