Refactoring toward frictionless & odorless codeGetting rid of globals
Originally posted at 3/30/2011
While I don’t really mind having global state, it tends to bite you on the ass eventually, so let us try to deal with this guy, shall we?
public class NHibernateActionFilter : ActionFilterAttribute { private static readonly ISessionFactory sessionFactory = BuildSessionFactory(); public static ISession CurrentSession { get { return HttpContext.Current.Items["NHibernateSession"] as ISession; } set { HttpContext.Current.Items["NHibernateSession"] = value; } } private static ISessionFactory BuildSessionFactory() { return new Configuration() .Configure() .BuildSessionFactory(); } public override void OnActionExecuting(ActionExecutingContext filterContext) { CurrentSession = sessionFactory.OpenSession(); } public override void OnActionExecuted(ActionExecutedContext filterContext) { var session = CurrentSession; if (session != null) { session.Dispose(); } } }
One easy way to do so would be to put the session directly where we want it to be, in the controller. We already have an extension point for that, the SessionController. Instead of referencing the global session, it can just hold its own:
public class SessionController : Controller { public HttpSessionStateBase HttpSession { get { return base.Session; } } public new ISession Session { get; set; } }
Which leads us to:
public class NHibernateActionFilter : ActionFilterAttribute { private static readonly ISessionFactory sessionFactory = BuildSessionFactory(); private static ISessionFactory BuildSessionFactory() { return new Configuration() .Configure() .BuildSessionFactory(); } public override void OnActionExecuting(ActionExecutingContext filterContext) { var sessionController = filterContext.Controller as SessionController; if (sessionController == null) return; sessionController.Session = sessionFactory.OpenSession(); } public override void OnActionExecuted(ActionExecutedContext filterContext) { var sessionController = filterContext.Controller as SessionController; if (sessionController == null) return; var session = sessionController.Session; if (session == null) return; session.Dispose(); } }
Now we have absolutely no global state, and yet we have a very easy access to the current session.
There are still a few things missing, can you see them?
More posts in "Refactoring toward frictionless & odorless code" series:
- (12 Apr 2011) What about transactions?
- (11 Apr 2011) Getting rid of globals
- (10 Apr 2011) The case for the view model
- (09 Apr 2011) A broken home (controller)
- (08 Apr 2011) Limiting session scope
- (07 Apr 2011) Hiding global state
- (06 Apr 2011) The baseline
Comments
The attribute should probably create/commit a transaction too.
Yes:
the session is injected via a property, constructor is much nicer.
the session factory is hold in a filter and I don't know if the filter is instantiated once per application (it should)
Yeah, the session factory is hold in a static field, so it's singleton, but it cannot be used anywhere else (for instance if I need StatelessSession or evicition of some cache regions). It should be a singleton in a container.
Is this series going to end up with a constuctor injected (Func <isession getOrOpenSessionForCurrentRequest*)?
Have you considered using a custom "ActionInvoker" in your controller instead ?
@Harry M
It should end with an ISession injected right to a controller constructor (or the action itself :>). No current resolver needed.
Why we can't simply inject session directly to constructor? What the point of custom filter?
Maybe DI is not cool anymore @ ALT .NET.
It always does bite afterthought IMHO
@Scooletz i was thinking of the situation that you are calling an action that doesn't hit the database, but there are actions on the controller that do. With the lazy Func <isession you wouldn't have the overhead of opening a transaction.
Could get really funky with a lazy proxyied ISession and keep the code even cleaner I guess.
Comment preview