IQueryable Repository with StructureMap (IoC) - How do i Implement IDisposable?

Question

If i have the following Repository:

public IQueryable<User> Users()
{
   var db = new SqlDataContext();
   return db.Users;
}

I understand that the connection is opened only when the query is fired:

public class ServiceLayer
{
   public IRepository repo;

   public ServiceLayer(IRepository injectedRepo)
   {
       this.repo = injectedRepo;
   }

   public List<User> GetUsers()
   {
       return repo.Users().ToList(); // connection opened, query fired, connection closed. (or is it??)
   }
}

If this is the case, do i still need to make my Repository implement IDisposable?

The Visual Studio Code Metrics certainly think i should.

I'm using IQueryable because i give control of the queries to my service layer (filters, paging, etc), so please no architectural discussions over the fact that im using it.

BTW - SqlDataContext is my custom class which extends Entity Framework's ObjectContext class (so i can have POCO parties).

So the question - do i really HAVE to implement IDisposable?

If so, i have no idea how this is possible, as each method shares the same repository instance.

EDIT

I'm using Depedency Injection (StructureMap) to inject the concrete repository into the service layer. This pattern is followed down the app stack - i'm using ASP.NET MVC and the concrete service is injected into the Controllers.

In other words:

1. User requests URL
2. Controller instance is created, which receives a new ServiceLayer instance, which is created with a new Repository instance.
3. Controller calls methods on service (all calls use same Repository instance)
4. Once request is served, controller is gone.

I am using Hybrid mode to inject dependencies into my controllers, which according to the StructureMap documentation cause the instances to be stored in the HttpContext.Current.Items.

So, i can't do this:

   using (var repo = new Repository())
   {
      return repo.Users().ToList();
   }

As this defeats the whole point of DI.

Rola

@Joshua Flanagan, @PHeiberg, @DanM, @Cottony - see my edit to my answer. Ayende Rahien (expert on this kind of stuff) has given the answer. Thought you might like to know. – Rola

+ 1 Really good question - was just asking this myself today (using StructureMap and Entity Framework 4). Wasn't sure if StructureMap auto-wrapped IDisposables inside a using() {} implicitly or if I need to implement. To be honest, still not sure from your answer. I'm using constructor injection and would love to find an example that covers the repository, service constructors and IDisposable implementation all together – Hartmunn

there is a method called "ReleaseAllScopedAndHttpVariables" or something - that you need to call from Application_EndRequest(). Use Hibernating Rhinos profiler to ensure everything gets disposed. – Rola

however that only ensures the objects are disposed after the request. if you want to dispose of something immediately, like i do - you need to call dispose. it does work - just use a profiler to make sure it does. – Rola

Answer 1

EDIT - Advice Received From Ayende Rahien

Got an email reply from Ayende Rahien (of Rhino Mocks, Raven, Hibernating Rhinos fame).

This is what he said:

You problem is that you initialize your context like this: _genericSqlServerContext = new GenericSqlServerContext(new EntityConnection("name=EFProfDemoEntities"));

That means that the context doesn't own the entity connection, which means that it doesn't dispose it. In general, it is vastly preferable to have the context create the connection. You can do that by using: _genericSqlServerContext = new GenericSqlServerContext("name=EFProfDemoEntities");

Which definetely makes sense - however i would have thought that Disposing of a SqlServerContext would also dispose of the underlying connection, guess i was wrong.

Anyway, that is the solution - now everything is getting disposed of properly.

So i no longer need to do using on the repository:

public ICollection<T> FindAll<T>(Expression<Func<T, bool>> predicate, int maxRows) where T : Foo
        {
            // dont need this anymore
            //using (var cr = ObjectFactory.GetInstance<IContentRepository>())
            return _fooRepository.Find().OfType<T>().Where(predicate).Take(maxRows).ToList();

And in my base repository, i implement IDisposable and simply do this:

Context.Dispose(); // Context is an instance of my custom sql context.

Hope that helps others out.

Rola

do you have lazy loading enabled? I'm having issues closing my connection since I have lazy loading enabled. – Pedagogy

@Pedagogy - nope, i disable it, and eager load what the consumer requires. You'll definetely have issues, especially if your running queries outside of the HTTP request thread (e.g in your MVC views). So be careful. – Rola

That is the best piece of advice I've received so far, so thank you! I did have several Views that referenced Services directly. I refactored those. However, I'm still running into issues. Anything that is lazy-loaded creates trouble. Are there ways around this? – Pedagogy

@Pedagogy - i suggest you ask a question on that, citing the specific issues you are having, including code samples. But yes, none of your Views should call into services. The most "logic" a View should do is HTML helpers - it should not call back to the service/controller. – Rola

Answer 2

I would say you definitely should. Unless Entity Framework handles connections very differently than LinqToSql (which is what I've been using), you should implement IDisposable whenever you are working with connections. It might be true that the connection automatically closes after your transaction successfully completes. But what happens if it doesn't complete successfully? Implementing IDisposable is a good safeguard for making sure you don't have any connections left open after your done with them. A simpler reason is that it's a best practice to implement IDisposable.

Implementation could be as simple as putting this in your repository class:

public void Dispose()
{
    SqlDataContext.Dispose();
}

Then, whenever you do anything with your repository (e.g., with your service layer), you just need to wrap everything in a using clause. You could do several "CRUD" operations within a single using clause, too, so you only dispose when you're all done.

Update

In my service layer (which I designed to work with LinqToSql, but hopefully this would apply to your situation), I do new up a new repository each time. To allow for testability, I have the dependency injector pass in a repository provider (instead of a repository instance). Each time I need a new repository, I wrap the call in a using statement, like this.

using (var repository = GetNewRepository())
{
    ...
}


public Repository<TDataContext, TEntity> GetNewRepository()
{
    return _repositoryProvider.GetNew<TDataContext, TEntity>();
}

If you do it this way, you can mock everything (so you can test your service layer in isolation), yet still make sure you are disposing of your connections properly.

If you really need to do multiple operations with a single repository, you can put something like this in your base service class:

public void ExecuteAndSave(Action<Repository<TDataContext, TEntity>> action)
{
    using (var repository = GetNewRepository())
    {
        action(repository);
        repository.Save();
    }
}

action can be a series of CRUD actions or a complex query, but you know if you call ExecuteAndSave(), when it's all done, you're repository will be disposed properly.

Kreiker

That's my point though - the service layer methods use a shared _repository instance. So how would i write the using statement in the method? I would need to new up the repository each time. The problem with this is im using DI to inject a repository into the constructor of my service layer. Hence the conundrum. =) – Rola

How do you handle multithreading with your shared repositories? – Cottony

@Cottony - its not a "shared" repository. The repository exists for the life of any HTTP Request. This is what StructureMap does - it says for each Http request, give the controller this service object, which in turn receives a repository object. The repository is shared amongst multple calls to the repository, but for only one HTTP Request - and therefore only one thread. – Rola

@Rola - ok. didn't know that :) What does structuremap do with the instances when the request goes out of scope? Does it check if any services implement IDisposable? – Cottony

@Cottony - when StructureMap "bootstraps" the instances, you can say the scope of the object. Most common is Hybrid - which causes StructureMap to stick the object in the HttpContext.Current.Items (or so i am led to believe). What i DONT know is if StructureMap also invokes the Dispose method on any types that implement IDisposable. Maybe i need to edit this question and take into account im using StructureMap to control instance scope. – Rola

@Rola - Yes. I would try to get StructureMap do invoke dispose and implement IDisposable in my repository classes. (and not use "using") – Cottony

@DanM - thanks for the update. I like your first idea, but how does that first bit of code work? What is _repositoryProvider? (and the GetNew method)? Is this code in your IoC container? Im not sure how this would work, because look at my Users() repository method - i need to new up a DB context to access the entities. It is this "newing" that i need to dispose of, not the actual repository instance. Man, this is tough. =) – Rola

@DanM - i used your answer as a basis for my answer. Thanks. (+1) – Rola

Answer 3

A common approach used with nhibernate is to create your session (ObjectContext) in begin_request (or some other similar lifecycle event) and then dispose it in end_request. You can put that code in an HttpModule.

You would need to change your Repository so that it has the ObjectContext injected. Your Repository should get out of the business of managing the ObjectContext lifecycle.

Skedaddle

Hmm, thanks for the alternative - but im not sure im comfortable with opening a new DB connection on every single HTTP Request. Every HTTP Request might not need a database call, so it would be wasting resources. When i work with database connections - i like to only open then when i need them (ie as late as possible). Thanks for the answer though. – Rola

@Joshua Flanagan - i just realised your part of the StructureMap project team, thanks for rocking along. =) I couldnt find any doco on "HttpContextBuildPolicy.DisposeAndClearAll();" which looks to do what i want. I have no idea where to put that code. – Rola

No matter, i found the solution (see my answer). Thanks tho. – Rola

@Rola - would a new connection be opened by just instantiating a context? I was under the impression that the connection would be opened/reused upon executing a query on the context. The drawback of creating a new context for each query is that you will potentially create a new connection for each database call. Best practice for database access in web seems to be connection per request. – Brandi

@Brandi - agreed. However, i didnt go with the above answer. Each time a user requests a page, a new controller is created, which creates a new service, which creates a new repository. Each "repository" instance is newed up with a SqlContext. This doesn't necessarily mean an "open" connection. It means a connection in sort of a "waiting" state. When i fire the query (.ToList()), it "opens" the connection, fires the query, and closes it. Still, i have sent emails to some experts asking what the correct solution is. – Rola

@Rola - Take a look at ayende.com/Blog/archive/2010/05/17/… for some of Oren's guidance. – Brandi

@Brandi - thanks for the link, very interesting. However, that article seems to be targeted at reducing round trips, and optimizing queries (Hibernating Rhinos). Whilst i am interesting in this, i am more interesting in how to design the intial architecture of StructureMap/IQueryable repos. A colleague of mine has posted a question on the EntityFramework forums - hopefully they come back with something. I will post an update here once they come back (if they do) – Rola

@Rola - The article doesn't show any example architecture, but points out why it's generally not a good idea to have multiple contexts for one request (multiple connections, no shared unit of work, multiple roundtrips, distributed transactions, etc). I'm looking forward to seeing any guidance you manage to get hold of. – Brandi