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: 5,955 | Comments: 44,412

filter by tags archive

Refactoring toward frictionless & odorless codeThe baseline


Originally posted at 3/30/2011

This is part of an exercise that I give in my course. The context is an MVC3 ASP.Net application. Here is how we start things:

public class MvcApplication : System.Web.HttpApplication
{
    private static readonly ISessionFactory sessionFactory = BuildSessionFactory();

    public static ISession CurrentSession
    {
        get{ return HttpContext.Current.Items["NHibernateSession"] as ISession;}
        set { HttpContext.Current.Items["NHibernateSession"] = value; }
    }

    public MvcApplication()
    {
        BeginRequest += (sender, args) =>
        {
            CurrentSession = sessionFactory.OpenSession();
        };
        EndRequest += (o, eventArgs) =>
        {
            var session = CurrentSession;
            if (session != null)
            {
                session.Dispose();
            }
        };
    }

    protected void Application_Start()
    {
        AreaRegistration.RegisterAllAreas();

        RegisterGlobalFilters(GlobalFilters.Filters);
        RegisterRoutes(RouteTable.Routes);
    }

    private static ISessionFactory BuildSessionFactory()
    {
        return new Configuration()
            .Configure()
            .BuildSessionFactory();
    }
}

And in the controller, we have something like this:

public class HomeController : SessionController
{
    public ActionResult Blog(int id)
    {
        var blog = MvcApplication.CurrentSession.Get<Blog>(id);

        return Json(blog, JsonRequestBehavior.AllowGet);
    }
}

This code is valid, it works, it follows best practices and I actually recommend using something very similar here.

It also annoyed me when I wrote it now, enough to write a series of blog posts detailing how to fix this.

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

Robert M&#252;hsig

Is this code still unit-testable? HttpContext and the static seems to be a problem if you try to write a test for the controller action. How do you solve this?

Thanks,

Robert

Chris Hynes

I'm very interested to see what you recommend. With your recent rantings on the demise of the Repository pattern, I looked at your code samples. The only things I could find were client projects, which require rather different architectures than web. MVC3 + NHibernate3 is an excellent combo.

Scott White

In other words Open Session in View (OSIV). The next step I assume will be to configure NHibernate to use web session scope?

Glenn Morton

Is there much of an overhead opening a session for every request that hits the application - feels a little excessive. Would it not be better to place an filter attribute on the controller to restrict the creation of sessions to only requests that require it?

Ryan McDonald

In the case of a RavenDB session object, how could you provide a database name to the call to OpenSessoin() in BeginRequest ?

Tim
Tim

Can't wait for the rest of this series.

Ayende Rahien

Ryan,

Relevance for the post?

There are ways to handle that, based on however you determine which tenant you are in, but it isn't relevant for this scenario

jhigh2000

As you say, It's really not bad at all, but you can definitely tuck some of the noise away by using a global action filter and your own base controller to manage your ISession.

alexander

You are crazy idiot. This code is holy shit!

Josh (@rebootd)

Curious to see the upcoming series. I use code a lot like this in my MS MVC3 app(s).

joe
joe

There might be some overhead if running that in Cassini, which runs every request (including static content) through asp.net.

I expect to see a lazy loaded UnitOfWork in the next post :)

Tyler Burd

Generally I prefer taking a direct dependency on ISession rather than using some sort of "CurrentSession" accessor. Easy enough to set up ISession as a service in Windsor, at least, with a PerWebRequest scope.

Adam B

I like it, but I was using the castle nhibernate facility and replacting that with this broke my app. Some requests used multiple sessions if certain filters were being invoked. Do you think that's actually a problem with my code? Should all requests come under one session?

Ayende Rahien

Adam,

In general, yes, a single session for the entire request is the way to go.

As for replacing this. Please wait, we have much better coming up

Rob Kent

It's nice and simple. I have two comments though.

In MVC 2, at least, we get a BeginRequest for every static resource, including images, Javascript, and CSS files. Maybe there is little overhead to opening an NH Session and I could be being excessively defensive, but I wrote a routine to check that we were processing an ASP request before I did anything like authenticating the user or opening an ISession.

Secondly, do you always need an ISession? I prefer to use a simple 'repository' that opens the session on demand and uses NH CurrentSessionContext.

Ayende Rahien

There is no cost for opening a new NH session.

And yes, you always want a session available, see my previous posts about why I dislike the repo pattern

Scooletz

A IoC container is the way to go. That's all about this code

Jimmy Zimms

I'm not a fan of singletons (well not container managed) and especially the infamous HttpContext.Current property but this is one place where when it does make sense to go down that route to leverage MSR Moles for testing purposes. Again, I usually try to design around that but there's times when it's just the best approach when complexity vs "purity" is balanced out in a solution.

Comment preview

Comments have been closed on this topic.

FUTURE POSTS

  1. What is new in RavenDB 3.5–Intro - 11 hours from now
  2. Production postmortem: The case of the infected cluster - about one day from now

There are posts all the way to Jul 09, 2015

RECENT SERIES

  1. The RavenDB Comic Strip (3):
    28 May 2015 - Part III – High availability & sleeping soundly
  2. Special Offer (2):
    27 May 2015 - 29% discount for all our products
  3. RavenDB Sharding (3):
    22 May 2015 - Adding a new shard to an existing cluster, splitting the shard
  4. Challenge (45):
    28 Apr 2015 - What is the meaning of this change?
  5. Interview question (2):
    30 Mar 2015 - fix the index
View all series

Syndication

Main feed Feed Stats
Comments feed   Comments Feed Stats