3

I'm fairly new at using dependency injection and I think I must be overlooking something really simple.

I have a Web API project where I'm registering generic repositories. The repositories take a dbContext as a parameter in their constructor.

The behavior I find strange is that I can make one successfull call to the service but any subsequent calls tell me that the dbcontext has been disposed. I do have a using statement in there but that shouldn't be a problem since DI is supposed to be creating new instances of my dependencies for each web request(although I could be wrong).

Here is my generic repository:

 public class GenericRepository<T> : IGenericRepository<T> where T : class
{
    internal DbContext _context;
    internal DbSet<T> _dbSet;
    private bool disposed;

    public GenericRepository(DbContext context)
    {
        _context = context;
        _dbSet = _context.Set<T>();
    }

    /// <summary>
    /// This constructor will set the database of the repository 
    /// to the one indicated by the "database" parameter
    /// </summary>
    /// <param name="context"></param>
    /// <param name="database"></param>       
    public GenericRepository(string database = null)
    {
        SetDatabase(database);
    }

    public void SetDatabase(string database)
    {
        var dbConnection = _context.Database.Connection;
        if (string.IsNullOrEmpty(database) || dbConnection.Database == database)
            return;

        if (dbConnection.State == ConnectionState.Closed)
            dbConnection.Open();

        _context.Database.Connection.ChangeDatabase(database);
    }

    public virtual IQueryable<T> Get()
    {
        return _dbSet;
    }

    public virtual T GetById(object id)
    {
        return _dbSet.Find(id);
    }

    public virtual void Insert(T entity)
    {
        _dbSet.Add(entity);
    }

    public virtual void Delete(object id)
    {
        T entityToDelete = _dbSet.Find(id);
        Delete(entityToDelete);
    }

    public virtual void Delete(T entityToDelete)
    {
        if (_context.Entry(entityToDelete).State == EntityState.Detached)
        {
            _dbSet.Attach(entityToDelete);
        }

        _dbSet.Remove(entityToDelete);
    }

    public virtual void Update(T entityToUpdate)
    {
        _dbSet.Attach(entityToUpdate);
        _context.Entry(entityToUpdate).State = EntityState.Modified;
    }

    public virtual void Save()
    {
        _context.SaveChanges();
    }
    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposed)
            return;

        if (disposing)
        {
            //free managed objects here
            _context.Dispose();
        }

        //free any unmanaged objects here
        disposed = true;
    }

    ~GenericRepository()
    {
        Dispose(false);
    }
}

Here is my generic repository interface:

 public interface IGenericRepository<T> : IDisposable where T : class
{
    void SetDatabase(string database);
    IQueryable<T> Get();       
    T GetById(object id);
    void Insert(T entity);
    void Delete(object id);
    void Delete(T entityToDelete);
    void Update(T entityToUpdate);
    void Save();
}

This is my WebApiConfig:

public static class WebApiConfig
{
    public static void Register(HttpConfiguration config)
    {
        // Web API configuration and services
        var container = new UnityContainer();

        container.RegisterType<IGenericRepository<Cat>, GenericRepository<Cat>>(new HierarchicalLifetimeManager(), new InjectionConstructor(new AnimalEntities()));
        container.RegisterType<IGenericRepository<Dog>, GenericRepository<Dog>>(new HierarchicalLifetimeManager(), new InjectionConstructor(new AnimalEntities()));           

        config.DependencyResolver = new UnityResolver(container);

        config.Formatters.JsonFormatter.SupportedMediaTypes.Add(new MediaTypeHeaderValue("text/html"));

        // Web API routes
        config.MapHttpAttributeRoutes();

        config.Routes.MapHttpRoute(
            name: "DefaultApi",
            routeTemplate: "api/{controller}/{id}",
            defaults: new { id = RouteParameter.Optional }
        );
    }
}

This is my DependencyResolver(which is pretty standard):

public class UnityResolver : IDependencyResolver
{
    protected IUnityContainer container;

    public UnityResolver(IUnityContainer container)
    {
        this.container = container ?? throw new ArgumentNullException(nameof(container));
    }

    public object GetService(Type serviceType)
    {
        try
        {
            return container.Resolve(serviceType);
        }
        catch (ResolutionFailedException)
        {
            return null;
        }
    }

    public IEnumerable<object> GetServices(Type serviceType)
    {
        try
        {
            return container.ResolveAll(serviceType);
        }
        catch (ResolutionFailedException)
        {
            return new List<object>();
        }
    }

    public IDependencyScope BeginScope()
    {
        var child = container.CreateChildContainer();
        return new UnityResolver(child);
    }

    public void Dispose()
    {
        Dispose(true);
    }

    protected virtual void Dispose(bool disposing)
    {
        container.Dispose();
    }
}

And finally this is part of the controller that's giving me trouble:

public class AnimalController : ApiController
{
    private readonly IGenericRepository<Cat> _catRepo;
    private readonly IGenericRepository<Dog> _dogPackRepo;

    public AnimalController(IGenericRepository<Cat> catRepository,
        IGenericRepository<Dog> dogRepository)
    {
        _catRepo = catRepository;
        _dogRepo = dogRepository;
    }

    [HttpGet]
    public AnimalDetails GetAnimalDetails(int tagId)
    {
        var animalDetails = new animalDetails();

        try
        {
            var dbName = getAnimalName(tagId);

            if (dbName == null)
            {
                animalDetails.ErrorMessage = $"Could not find animal name for tag Id {tagId}";
                return animalDetails;
            }

        }
        catch (Exception ex)
        {
            //todo: add logging
            Console.WriteLine(ex.Message);
            animalDetails.ErrorMessage = ex.Message;
            return animalDetails;
        }

        return animalDetails;
    }

    private string getAnimalName(int tagId)
    {
        try
        {
            //todo: fix DI so dbcontext is created on each call to the controller
            using (_catRepo)
            {
                return _catRepo.Get().Where(s => s.TagId == tagId.ToString()).SingleOrDefault();
            }
        }
        catch (Exception e)
        {
            //todo: add logging
            Console.WriteLine(e);
            throw;
        }
    }       
}

The using statement around the _catRepo object is not behaving as expected. After I make the first service call the _catRepo is disposed of. On a subsequent call I'm expecting to have a new _catRepo instantiated. However, this is not the case since the error I'm getting talks about the dbcontext being disposed.

I've tried changing the LifeTimeManager to some of the other ones available but that didn't help.

I also started going down a different route where the generic repository would take a second generic class and instantiate its own dbcontext from it. However, when I did that Unity couldn't find my controller's single-parameter constructor.

I guess what I really need, per the comments below, is a way to instantiate the DbContext on a per-request basis. I don't know how to go about doing that though.

Any hints would be greatly appreciated.

Bruno
  • 533
  • 1
  • 6
  • 27
  • 1
    When you do `using (_catRepo)`, at the end of the call the repo is disposed, which disposes the db context. Don't put a `using` block there. Your DI container should be able to configure db contexts to be per-request (not sure how this works in Unity), but you don't need to call `repo.Dispose()` if this is set up correctly – Dan Oct 04 '17 at 20:18
  • Isn't a subsequent call to the web service supposed to instantiate a new _catRepo? I guess I must be missing something in the setup... – Bruno Oct 04 '17 at 20:19
  • 4
    The DI container is supposed to dispose of resources for you. You shouldn't have to do it manually. – Kyle Oct 04 '17 at 20:21
  • 1
    You are creating your context only when the application starts, so if the first request happens and you dispose the db, the next request not has more a db to use. Try change it to create and dispose db per request. – Renato Leite Oct 04 '17 at 20:24
  • 1
    Also, I went down this rabbit hole before: I would not recommend generic repositories. Use specific repositories. you'll thank me later when you need domain-specific actions/transactions etc that are not CRUD – Dan Oct 04 '17 at 20:26
  • You should read [this](https://stackoverflow.com/questions/17943870/decorators-and-idisposable) and [this](https://stackoverflow.com/questions/30283564/dependency-injection-and-idisposable). – Steven Oct 04 '17 at 21:16
  • Not sure why IRepository implements IDisposable. The container will dispose itself due to HierarchicalLifetimeManager. Remove the disposable and your issue should go away. My reference is the current API I have designed that has 3M hits a month. –  Oct 10 '17 at 15:18

3 Answers3

4

Let's take a look at your registration:

container.RegisterType<IGenericRepository<Cat>, GenericRepository<Cat>>(
    new HierarchicalLifetimeManager(), 
    new InjectionConstructor(new AnimalEntities()));

container.RegisterType<IGenericRepository<Dog>, GenericRepository<Dog>>(
    new HierarchicalLifetimeManager(), 
    new InjectionConstructor(new AnimalEntities()));

You are creating two instances of AnimalEntities at startup, but those instances are reused for the duration of the whole application. This is a terrible idea. You probably intend to have one DbContext per request, but the instance wrapped by the InjectionConstructor is a constant.

You should change your configuration to the following:

container.RegisterType<IGenericRepository<Cat>, GenericRepository<Cat>>(
    new HierarchicalLifetimeManager());

container.RegisterType<IGenericRepository<Dog>, GenericRepository<Dog>>(
    new HierarchicalLifetimeManager());

// Separate 'scoped' registration for AnimalEntities.
container.Register<AnimalEntities>(
    new HierarchicalLifetimeManager()
    new InjectionFactory(c => new AnimalEntities()));

This is much simpler and now the AnimalEntities is registered as 'scoped' as well.

What's nice about this is that Unity will now dispose your AnimalEntities once the scope (the web request) ends. This prevents you from having to implement IDisposable on consumers of AnimalEntities, as explained here and here.

NightOwl888
  • 55,572
  • 24
  • 139
  • 212
Steven
  • 166,672
  • 24
  • 332
  • 435
  • thanks for giving such a detailed answer. You are correct in that my intention was to have one DbContext per request. Unfortunately, your suggestion is not working for me. It looks like my controller's constructor cannot be found since I'm getting this error msg: "An error occurred when trying to create a controller of type 'AnimalController'. Make sure that the controller has a parameterless public constructor.". Was I supposed to change my controller's signature? – Bruno Oct 05 '17 at 15:14
  • parameterless ctor is because your registration is not correctly bound. you should be able to dig into the registrations container to see the error messages within the Unity container. I would recommend putting this new registration code into a TEST before attempting to resolve in a multi-parameter controller.. –  Oct 10 '17 at 15:23
0

I figured out what was going on. As several people have indicated, my repository doesn't need to inherit from IDisposable since the Unity container will dispose of these repositories when the time is right. However, that wasn't the root of my problems.

The main challenge to overcome was getting one dbContext per request. My IGenericRepository interface has stayed the same but my GenericRepository implemenation now looks like this:

public class GenericRepository<TDbSet, TDbContext> : 
    IGenericRepository<TDbSet> where TDbSet : class
    where TDbContext : DbContext, new()
{
    internal DbContext _context;
    internal DbSet<TDbSet> _dbSet;

    public GenericRepository(DbContext context)
    {
        _context = context;
        _dbSet = _context.Set<TDbSet>();
    }

    public GenericRepository() : this(new TDbContext())
    {
    }

    /// <summary>
    /// This constructor will set the database of the repository 
    /// to the one indicated by the "database" parameter
    /// </summary>
    /// <param name="context"></param>
    /// <param name="database"></param>       
    public GenericRepository(string database = null)
    {
        SetDatabase(database);
    }

    public void SetDatabase(string database)
    {
        var dbConnection = _context.Database.Connection;
        if (string.IsNullOrEmpty(database) || dbConnection.Database == database)
            return;

        if (dbConnection.State == ConnectionState.Closed)
            dbConnection.Open();

        _context.Database.Connection.ChangeDatabase(database);
    }

    public virtual IQueryable<TDbSet> Get()
    {
        return _dbSet;
    }

    public virtual TDbSet GetById(object id)
    {
        return _dbSet.Find(id);
    }

    public virtual void Insert(TDbSet entity)
    {
        _dbSet.Add(entity);
    }

    public virtual void Delete(object id)
    {
        TDbSet entityToDelete = _dbSet.Find(id);
        Delete(entityToDelete);
    }

    public virtual void Delete(TDbSet entityToDelete)
    {
        if (_context.Entry(entityToDelete).State == EntityState.Detached)
        {
            _dbSet.Attach(entityToDelete);
        }

        _dbSet.Remove(entityToDelete);
    }

    public virtual void Update(TDbSet entityToUpdate)
    {
        _dbSet.Attach(entityToUpdate);
        _context.Entry(entityToUpdate).State = EntityState.Modified;
    }

    public virtual void Save()
    {
        _context.SaveChanges();
    }
}

The default constructor is now responsible for creating a new DbContext of the type specified when the class is instantiated(I actually have more than one type of DbContext in my application). This allows for a new DbContext to be created for every web request. I tested this by using the using statement in my original repository implementation. I was able to verify that I no longer get the exception about the DbContext being disposed on subsequent requests.

My WebApiConfig now looks like this:

 public static class WebApiConfig
{
    public static void Register(HttpConfiguration config)
    {
        // Web API configuration and services
        var container = new UnityContainer();

        container.RegisterType<IGenericRepository<Cat>, GenericRepository<Cat, AnimalEntities>>(new HierarchicalLifetimeManager(), new InjectionConstructor());
        container.RegisterType<IGenericRepository<Dog>, GenericRepository<Dog, AnimalEntities>>(new HierarchicalLifetimeManager(), new InjectionConstructor());                                  

        config.DependencyResolver = new UnityResolver(container);

        config.Formatters.JsonFormatter.SupportedMediaTypes.Add(new MediaTypeHeaderValue("text/html"));

        // Web API routes
        config.MapHttpAttributeRoutes();

        config.Routes.MapHttpRoute(
            name: "DefaultApi",
            routeTemplate: "api/{controller}/{id}",
            defaults: new { id = RouteParameter.Optional }
        );
    }
}

One thing that was causing me a lot of pain here is that I didn't realize I still had to call the InjectionConstructor in order to use the parameterless constructor of the repository class. Not including the InjectionConstructor was causing me to get the error about my controller's constructor not being found.

Once I got over that hurdle I was able to change my controller what I have below. The main difference here is that I'm no longer using using statments:

public class IntegrationController : ApiController
{
    private readonly IGenericRepository<Cat> _catRepo;
    private readonly IGenericRepository<Dog> _dogPackRepo;

    public IntegrationController(IGenericRepository<Cat> catRepository,
        IGenericRepository<Dog> dogRepository)
    {
        _catRepo = catRepository;
        _dogRepo = dogRepository;
    }

[HttpGet]
public AnimalDetails GetAnimalDetails(int tagId)
{
    var animalDetails = new animalDetails();

    try
    {
        var dbName = getAnimalName(tagId);

        if (dbName == null)
        {
            animalDetails.ErrorMessage = $"Could not find animal name for tag Id {tagId}";
            return animalDetails;
        }
    }
    catch (Exception ex)
    {
        //todo: add logging
        Console.WriteLine(ex.Message);
        animalDetails.ErrorMessage = ex.Message;
        return animalDetails;
    }

    return animalDetails;
}

private string getAnimalName(int tagId)
{
    try
    {            
         return _catRepo.Get().Where(s => s.TagId == 
           tagId.ToString()).SingleOrDefault();            
    }
    catch (Exception e)
    {
        //todo: add logging
        Console.WriteLine(e);
        throw;
    }
}       
}
Bruno
  • 533
  • 1
  • 6
  • 27
0

The way I solved my problem is a little different than what these answers suggest.

I'm in an MVC app but the logic should be similar for this.

As others have stated, creating an instance of an object inside and InjectionContructor essentially creates a static copy of that instance that is used for all future instances of the resolving type. To fix this, I simply register the context as a type and then let Unity resolve the context when it resolves the service. By default, it creates a new instance each time:

UnityConfig:

public static void RegisterComponents()
{
    var container = new UnityContainer();

    container.RegisterType<PrimaryContext>(new InjectionConstructor());
    container.RegisterType<LocationService>();

    DependencyResolver.SetResolver(new UnityDependencyResolver(container));
}

PrimaryContext:

//Allows for a default value if none is passed
public PrimaryContext() : base(Settings.Default.db) { }
public PrimaryContext(string connection) : base(connection)
{
}

LocationService:

PrimaryContext _context;
public LocationService(PrimaryContext context)
{
    _context = context;
}

I can't give a ton of specifics about exactly how it works, but this appears to have fixed the problem I was having (I was getting the same error message) and it's super simple.

Eric Aya
  • 69,473
  • 35
  • 181
  • 253
Tyler Sells
  • 463
  • 4
  • 16