Finding Bad Controllers

asp.net, asp.net mvc, code 0 comments suggest edit

In one mailing list I’m on, someone ran into a problem where they renamed a controller, but ASP.NET MVC could not for the life of it find it. They double checked everything. But ASP.NET MVC simply reported a 404.

This is usually where I tell folks to run the following NuGet command:

Install-Package RouteDebugger

RouteDebugger is a great way to find out why a route isn’t matching a controller.

In this particular case, the culprit was that the person renamed the controller and forgot to append the “Controller” suffix. So from something like HomeController to Default.

In this post, I’ll talk a little bit about what makes a controller a controller and a potential solution so this mistake is caught automatically.

What’s with the convention anyways?

A question that often comes up is why require a name based convention for controllers in the first place? After all, controllers also must implement the IController interface and that should be enough to indicate the class is a controller.

Great question! That decision was made before I joined Microsoft and I never bothered to ask why. What I did do was make up my own retroactive reasoning, which is as follows.

Suppose we simply took the class name to be the controller name. Also suppose you’re building a page to display a category and you want the URL to be something like /category/unicorns. So you write a controller named Category. Chances are, you also have an entity class named Category too that you want to use within the Category controller. So now, a common default situation becomes painful.

If I could get in a time machine and revisit this decision, would I? Hell no! As my former co-worker Eilon always says; if you have a time machine, there’s probably a lot better things you can do than fix bad design decisions in ASP.NET MVC.

But if I were to do this again, I’m not so sure I’d require the “Controller” suffix. Instead, I’d suggest using plural controllers (and URLs) to better avoid conflicts. So the controller would be Categories and the URL would be /categories/unicorns. And perhaps I’d make the “Controller” suffix allowed as a way to resolve conflicts. So CategoryController would still work fine (as a conroller named “category”) if that’s the way you roll.

How do I detect Controller Problems?

Since I didn’t use my time machine to fix this issue (I would rather use it to go back in time and fix line endings text encodings), what can we do about it?

The simplest solution is to do nothing. How often will you make this mistake?

Then again, when you do, it’s kind of maddening because there’s no error message. The controller just isn’t there. Also, there’s other mistakes you could make, though many are unlikley. All the following look like they were intended to be controllers, but aren’t.

public class Category : Controller
{}

public abstract class CategoryContoller : Controller
{}

public class Foo 
{
    public class CategoryController : Controller 
    { }
}

class BarController : Controller
{}

internal class BarController : Controller
{}

public class BuzzController
{}

Controllers in ASP.NET MVC must be public, non-abstract, non-nested, and implement IController, or derive from a class that does.

So the list of possible mistakes you might make are:

  • Forget to add the “Controller” suffix (happens more than you think)
  • Make the class abstract (probably not likely)
  • Nest the class (could happen by accident like you thought you were pasting in a namespace, but very very unlikely)
  • Forget to make the class public (not likely if you use the Add Controller dialog. But if you use the Add Class dialog, could happen)
  • Forget to derive from Controller or ControllerBase or implement IController. (Again, probably not very likely)

How to detect these cases

As I mentioned before, it might not be that important to do this, but one thing you could consider is writing a unit test that detects these various conditions. Well how would you do that?

You’re in luck. Whether this is super useful or not, I still found this to be an interesting problem to solve. I wrote a ControllerValidator class with some methods for finding all controllers that match one of these conditions.

It builds on the extension method to retrieve all types I blogged about the other day. First, I wrote extension methods for the various controller conditions:

static bool IsPublicClass(this Type type)
{
    return (type != null && type.IsPublic && type.IsClass && !type.IsAbstract);
}

static bool IsControllerType(this Type t)
{
    return typeof (IController).IsAssignableFrom(t);
}

static bool MeetsConvention(this Type t)
{
    return t.Name.EndsWith("Controller", StringComparison.OrdinalIgnoreCase);
}

With these methods, it became a simple case of writing methods that checked for two out of these three conditions.

For example, to get all the controllers that don’t have the “Controller” suffix:

public static IEnumerable<Type> GetUnconventionalControllers
  (this Assembly assembly)
{
  return from t in assembly.GetLoadableTypes()
      where t.IsPublicClass() && t.IsControllerType() && !t.MeetsConvention()
      select t;
}

With these methods, it’s a simple matter to write automated tests that look for these mistakes.

Source code and Demo

I added these methods as part of the ControllerInspector library which is available on NuGet. You can also grab the source code from my CodeHaacks repository on GitHub (click the Clone in Windows button!).

If you get the source code, check out the following projects:

  • ControllerInspectorTests.csproj – Unit tests of these new methods show you how you might write your own unit tests.
  • MvcHaack.ControllerInspector.csproj – Contains the ControllerValidator class.
  • MvcHaack.ControllerInspector.DemoWeb.csproj – Has a website that demonstrates this class too.

The demo website’s homepage uses these methods to show a list of bad controllers.

bad-controllers

The code I wrote is based on looking at how ASP.NET MVC locates and determines controllers. It turns out, because of the performance optimizations, it takes a bit of digging to find the right code.

If you’re interested in looking at the source, check out TypeCacheUtil.cs on CodePlex. It’s really nice that ASP.NET MVC is not only open source, but also accepts contributions. I highly recommend digging through the source as there’s a lot of interesting useful code in there, especially around reflection.

If you don’t find this useful, I hope you at least found it illuminating.

Found a typo or error? Suggest an edit! If accepted, your contribution is listed automatically here.

Comments

avatar

19 responses

  1. Avatar for Pieter Germishuys
    Pieter Germishuys July 25th, 2012

    Great stuff Phil

  2. Avatar for Bob
    Bob July 25th, 2012

    Or just use T4MVC. I hope someday T4MVC became part of the official ASP.NET MVC release.

  3. Avatar for Dan Ormisher
    Dan Ormisher July 25th, 2012

    Funny seeing this post Phil - me and my colleagues were discussing this convention the other day after I got stuck for an hour wondering why my action method wasn't being hit. Turned out I had forgot the 'Controller' suffix. I posted this question on stackoverflow to see what other people thought of the convention too. I was annoyed by the proclamations of 'convention over configuration' which it clearly isn't!

  4. Avatar for Nicholas Murray
    Nicholas Murray July 25th, 2012

    "I would rather use it to go back in time and fix line endings text encodings", nice to see you would use the power of your time machine for good instead of evil!

  5. Avatar for Justin
    Justin July 25th, 2012

    Hey Phil, nice post!
    I don't run into frustrating situations like this often but it does happen occasionally. I'm not sure if I am more pissed off while trying to figure it out or after I realize how stupid the mistake was.
    Next time I'll give your NuGet package a try. Thanks.

  6. Avatar for James Culbertson
    James Culbertson July 25th, 2012

    Another case for bad controllers is when the controller root name matches the name of a folder on the website, for example "ContentController". This will also fail silently and is frustrating to troubleshoot.
    Why in ASP.NET MVC 3 the default route is not working for a controller called ContentController?

  7. Avatar for Anonymous
    Anonymous July 25th, 2012

    Thanks Phil. Now the next time someone asks me why their controller isn't working I'll have that first easy question to ask before I go to their desk. "Is it plugged in... at both ends?"

  8. Avatar for David McClelland
    David McClelland July 25th, 2012

    If you happen to use NDepend as part of a continuous integration process, you could automagically detect this with a CQL or CQLinq query and enforce it as a build policy.

  9. Avatar for Jeff LeBert
    Jeff LeBert July 25th, 2012

    This sounds more like an FxCop rule. Now that FxCop is in Visual Studio 2012 Professional named Code Analysis, we can all use it. David mentioned NDepend which is an incredible tool, but not free in Visual Studio.

  10. Avatar for Ruben Bartelink
    Ruben Bartelink July 25th, 2012

    For people that are interested in managing such things via tests instead of tools, the ControllerInspector concept is in the direction of Mark Seemann (@ploeh)'s AutoFixture's Idioms stuff, which I got introduced to via http://stackoverflow.com/a/11455580/11635
    Then engine for Idioms supports types so [some]one could easily add validation of conventions like this as an AF Idioms Contrib thing.
    In the MVC space, the sort of thing this could also be employed for is verifying conventions on Models (e.g. must have default constructor).
    The fun bit is figuring out a way of doing the APIs around this in a way that satisfies enough people (which is why Mark says he's yet to get to the stage where he's ready to publicise it).

  11. Avatar for Behtash Moradi
    Behtash Moradi July 26th, 2012

    Great stuff,
    Thanks
    Behtash

  12. Avatar for Vincent
    Vincent July 26th, 2012

    This is a very good post.
    Never happened to me but will keep it in my mind.

  13. Avatar for Jamie Gaines
    Jamie Gaines July 26th, 2012

    Thanks, Phil! What's the name of the mailing list you're on? I'm always looking for great new ASP.NET MVC resources.

  14. Avatar for Patrik Svensson
    Patrik Svensson July 30th, 2012

    Why not make this as a plugin to Glimpse? :)

  15. Avatar for terry
    terry August 16th, 2012

    Good stuff Phil this is truly useful stuff. There is one other failure mode that will cause a controller not to be found. If the namespace name in the controller source file doesn't derive from the root namespace it won't be found either unless namespace info is specifically attached to the route definition in the AreaRegistration class or the global.asax file.
    For example, if the root namespace for the project is "hello" and the namespace declared in the controller file within the same project is somehow changed to "hahello" then it will return a 404 even if all the attributes of the controller look fine.

  16. Avatar for haacked
    haacked August 20th, 2012

    Hi Terry, I was unable to repro the issue you described. Perhaps you meant when you have two controllers with the same name but different namespaces?

  17. Avatar for terry
    terry August 23rd, 2012

    Phil I want to add that you'll need to create a solution with the following properties:
    0.Be a C# project. (Since VB.NET handles namespaces a little different with default namespaces).
    1.Be a multi-project mvc 3 project.
    2.The primary project should have a Area defined in it.
    3.The second project should have a output type of class library.
    4.The namespace in the file of the secondary project should be different than the one found according to the values found under Properties -> Application -> AssemblyName and Default namespace in the secondary project. This will be the controller that will have a 404 issue.
    In particular the mvc runtime always tries to look relative to the project where the Area is defined before other projects.
    P.S. Another potential 404 issue I haven't seen much Microsoft documentation on is setting up views in a multi-project like this to avoid 404 errors.
    For example,by default the Build Action for the Views you add to your project have a default value of “Content”. You need to change this to “Embedded Resource” so the view will be embedded into the .dll and the route found after you ensure the .dll is being properly exported to <primary_projname_with_Area>\bin. This will happen when you add a MVC project outside of where the main “Area“ is defined in the primary project where the Area is defined.
    P.P.S Same type of problem with trying to get the runtime to properly resolve the location of JavaScript files in multi-project solutions where the project isn't the primary one with the Area. Although this was one issue I didn't get to fix and it didn't include a 404 error but it was failing to find the right file though.

  18. Avatar for terry
    terry August 23rd, 2012

    Meant to say:
    "4.The namespace in the file of the controller of the secondary project..."

  19. Avatar for byhard
    byhard December 7th, 2012

    funy,thanks.