Here’s some code I wrote:
public class MyBaseController : Controller { // ... public new ISession Session { get; private set; } // .... }
ISession is an interface type I wrote which exposes everything I store in the session at runtime. I use constructor injection to assign this to a simple type which maps to the "real" Session during application runtime, and a "mock" session for unit testing. Session, in ASP.NET, is a user-specific cache.
This hides Controller.Session (of type HttpSessionStateBase) and replaces it with a strongly-typed, mockable replacement. Normally I wouldn’t use public new, but in this specific case it seems like the best way. It stops developers from using the Web-dependent Session property in derived types and forces them to code to the strong-typed interface. But still, it’s public new, which seems yucky.
What do you think?
{ 10 } Comments
Personally, I don’t use anything on the Controller base class, and I wish the Session property wasn’t even available.
In my current application we treat the user session just like any other dependency, and require it in the constructor of the controller, seen below:
public ContactController(IContactRepository repository, IContactMapper mapper, IUserSession session)
This also allows me to easily unit test the controller by mocking the session interface and passing in different values during the tests — without having to worry about some fancy controllerbuilder session injection complexity.
And of course, the implementation of my IUserSession interface simply delegates the properties to HttpContext.Current.Session[] when the application is running in non-test mode.
-Matt
Every time I use the ‘new’ keyword I feel a bit dirty, not sure if that is right or not. I tend to agree with Matt, I generally create some kind of strongly typed IUserSettings interface that is passed via dependency injection.
Matt and Craig,
I’m doing exactly the same thing as Matt, except that I’m hiding/replacing the standard Controller.Session, instead of just adding yet another property.
In my last 3 asp.net mvc apps I did away with session altogether instead using Cookies to hold the data.
Cyril, cookies have their place, but Session is not that place. Cookies must go to the client, are untyped, make unit testing harder rather than easier, add bandwidth, and are hopelessly broken if the user disables cookies.
+1 to what Matt and Craig said.
And it’s not an extra property, it’s just a constructor argument passed in. Makes mocking and unit-testing super simple and allows you to easily switch where your session data is stored in the future (just implement a new IUserSession).
Patrick (and Matt and Craig): I must not have made myself clear. I’m already doing constructor injection for the same type of type-safe "IUserSession" interface that Matt demonstrated. I’ve tweaked the post to clarify this.
But when you do constructor injection you need to store that reference somewhere, right? You put it in a property or field of the class.
The question I’m asking in this post is: Given that we all agree that it’s a mistake to use the built-in Controller.Session property, does it make more sense to overwrite this property and use the same name for the injected "user session" then to put it in an entirely separate location?
My recommendation is never use ‘new’ keyword like that. It’s too error prone. It will be better just forget about it and tell anyone not to use it at all. Just introduce some new property and do whatever you want if you can’t easily use existing property without the ‘new’ keyword.
+1
It is not yucky. Using default Session property of Controller is yucky. Using this construction in base class is completely valid. As long as all controllers use the same ‘version’ of Session property, it is ok. I used to create SessionWrapper property with ISessionWrapper interface in base class, but I like this solution even more.
I agree with new keyword implementation causing errors.
Ray Akkanson
{ 1 } Trackback
[...] to VoteReplacing Controller.Session in ASP.NET MVC: Is This Wrong? (6/17/2010)Thursday, June 17, 2010 from Craig StuntzHere’s some code I wrote: public class MyBaseController : [...]
Post a Comment