Ayende @ Rahien

Refunds available at head office

Refactoring toward frictionless & odorless code: Getting 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?

Comments

Paul Stovell
04/11/2011 09:16 AM by
Paul Stovell

The attribute should probably create/commit a transaction too.

Scooletz
04/11/2011 09:16 AM by
Scooletz

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)

Scooletz
04/11/2011 09:18 AM by
Scooletz

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.

Harry M
04/11/2011 09:31 AM by
Harry M

Is this series going to end up with a constuctor injected (Func <isession getOrOpenSessionForCurrentRequest*)?

  • well maybe something with a shorter name
Guillaume
04/11/2011 09:33 AM by
Guillaume

Have you considered using a custom "ActionInvoker" in your controller instead ?

Scooletz
04/11/2011 11:31 AM by
Scooletz

@Harry M

It should end with an ISession injected right to a controller constructor (or the action itself :>). No current resolver needed.

Nick Sergeev
04/11/2011 05:40 PM by
Nick Sergeev

Why we can't simply inject session directly to constructor? What the point of custom filter?

Dmitry
04/12/2011 04:10 AM by
Dmitry

Maybe DI is not cool anymore @ ALT .NET.

Patrick Smacchia
04/12/2011 08:58 AM by
Patrick Smacchia

While I don’t really mind having global state, it tends to bite you on the ass eventually,

It always does bite afterthought IMHO

Harry M
04/12/2011 12:49 PM by
Harry M

@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.

Comments have been closed on this topic.