Ayende @ Rahien

Hi!
My name is Oren Eini
Founder of Hibernating Rhinos LTD and RavenDB.
You can reach me by phone or email:

ayende@ayende.com

+972 52-548-6969

, @ Q c

Posts: 10 | Comments: 33

filter by tags archive

Refactoring toward frictionless & odorless codeGetting rid of globals

time to read 5 min | 847 words

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:

  1. (12 Apr 2011) What about transactions?
  2. (11 Apr 2011) Getting rid of globals
  3. (10 Apr 2011) The case for the view model
  4. (09 Apr 2011) A broken home (controller)
  5. (08 Apr 2011) Limiting session scope
  6. (07 Apr 2011) Hiding global state
  7. (06 Apr 2011) The baseline

Comments

Paul Stovell

The attribute should probably create/commit a transaction too.

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

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

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

  • well maybe something with a shorter name
Guillaume

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

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

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

Dmitry

Maybe DI is not cool anymore @ ALT .NET.

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

@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

Comments have been closed on this topic.

FUTURE POSTS

  1. Production postmortem: The case of the memory eater and high load - 3 days from now
  2. Production postmortem: The case of the lying configuration file - 4 days from now
  3. Production postmortem: The industry at large - 5 days from now
  4. The insidious cost of allocations - 6 days from now
  5. Find the bug: The concurrent memory buster - 7 days from now

And 4 more posts are pending...

There are posts all the way to Sep 10, 2015

RECENT SERIES

  1. Find the bug (5):
    20 Apr 2011 - Why do I get a Null Reference Exception?
  2. Production postmortem (10):
    14 Aug 2015 - The case of the man in the middle
  3. What is new in RavenDB 3.5 (7):
    12 Aug 2015 - Monitoring support
  4. Career planning (6):
    24 Jul 2015 - The immortal choices aren't
View all series

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats