DRY-ing ASP.NET MVC actions: ApplicationController or Service?

Question

I'm writing an ASP.NET MVC 3 app and I'm finding myself writing this line rather often in my action methods:

var user = _session.Single<User>(u => u.UserName == User.Identity.Name);

(Obviously used in conjunction with the AuthorizeAttribute)

There are other things that get repeated quite often but this one is the most prominent and I end up having 3 actions next to each other, each needing to retrieve the authorized user.

So this needs DRY-ing up:

Should I write an ApplicationContoller from which all other controller inherit and expose a User property there or should I add this to my IAdminService and expose it as a method?
Is an ApplicationController something to avoid or to embrace in ASP.NET MVC?

Have you considered a [custom membership provider](http://mattwrock.com/post/2009/10/14/Implementing-custom-Membership-Provider-and-Role-Provider-for-Authinticating-ASPNET-MVC-Applications.aspx)? — Josiah Ruddell, Apr 08 '11 at 22:02
@Josiah - Absolutely not :) Surely not for this tiny bit of functionality, that would be massive overkill. Besides I have the "standard" membership provider in place and running smoothly, I would only be wasting time. — Sergi Papaseit, Apr 08 '11 at 22:10

score 3 · Accepted Answer · answered Apr 09 '11 at 06:48

3

If you are finding yourself repeating this logic then a custom model binder for the User type might help:

public class UserModelBinder : DefaultModelBinder
{
    private readonly ISession _session;
    public UserModelBinder(ISession session)
    {
        _session = session;
    }

    public override object BindModel(ControllerContext controllerContext, ModelBindingContext bindingContext)
    {
        var username = controllerContext.HttpContext.User.Identity.Name;
        return _session.Single<User>(u => u.UserName == username);
    }
}

and once you register the binder your controller action might look like this:

[Authorize]
public ActionResult Foo(User user)
{
    // ...
}

answered Apr 09 '11 at 06:48

Darin Dimitrov

1,023,142
271
3,287
2,928

I thought about mentioning a model binder... But don't you feel that it's a bit hackish? I mean using the action parameters for such a use? – Linkgoron Apr 09 '11 at 12:16
1

@Linkgoron, no, personally I don't find this approach *hackish*. – Darin Dimitrov Apr 09 '11 at 12:27
@Linkgoron - I don't think it's hackish either; rather neat actually. The only two "issues" I see is that 1) it forces you to pass the User as an argument every time and so the default route "{controller}/{action}/{id}" is not matched any more. "{controller}/{action}/{user}/{id}" might not be a bad option though. 2) It's only useful for action methods and not anywhere else in your controller, like private methods, without passing it as an argument again... – Sergi Papaseit Apr 09 '11 at 15:17
@SergiPapaseit - Your argument isn't valid. Darin's code doesn't affect the route. Based on your logic, an Action method accepting a model would need the model built into the route, which is not the case. – Jeff Reddy Feb 13 '12 at 17:57

Linkgoron · Answer 2 · 2011-04-09T02:54:10.143

As a person who doesn't really like controller super-types I would consider using Dependency Injection and use constructor injection to "inject" the user.

o/c this has some drawbacks. This means you'll have to a field for every use in your controller, and also have to create a binding in your IOC tool. This also assumes you're using an IOC container.

Regarding the other options:

Exposing it in the IAdminService gives you the added benefit of being availiable in other places, and not just in the Controller. So that's a plus. Just be certain that you don't clutter your interface too much.

Using it in a base controller is also tempting at first, but I've found that controller base types get bloated and mismanaged as more and more functionality is added, because there's no multiple inheritance and people need some of this and some of that... Things can get ugly. Not to mention that if you use the AsyncController you'll have two base-types to manage.

Basically, between your two option's I'd use the interface.

No matter what you do, you can still add a method to the interface and also abstract it behind a User property in a base Controller.

"o/c this has some drawbacks. This means you'll have to a field for every use in your controller, and also have to create a binding in your IOC tool. This also assumes you're using an IOC container." - This isn't true at all. He could simply new() up whatever class he needs in a base controller without DI at all. — John Farrell, Apr 09 '11 at 03:34

DRY-ing ASP.NET MVC actions: ApplicationController or Service?

2 Answers2