Disposing Objects in WebAPI

Question

User-574293449 posted

I am going to maintain one existing API implementation. When i look into the code i got that there are some issue with object dispose. 

The following are my Base Controller which acts as parent controller for all.

Base Controller

    [LoggingFilter]
    [System.Web.Http.Authorize]
    public abstract class BaseV1Controller : ApiController
    {
        private ModelFactoryV1 _modelFactoryV1;
        private MyDBContext __db;
        private MyLoggingService _loggingService;
        private int _customerId;

        protected string __IPAddress;
        protected ILogger __logger;
        protected const int PAGE_SIZE_NORMAL = 20;
        protected const int PAGE_SIZE_MEDIA = 2;
        // GET: Base
        protected string __loggingResourceName = "Undefined - base controller";
        private void InitLogger()
        {
           Log.Logger.ForContext<BaseV1Controller>();
        }

        protected MyDBContext _db
        {
            get { return __db; }
            set { __db = value; }
        }

        public BaseV1Controller()
        {
            IEnumerable<string> values;
            __db = new MyDBContext();
            _loggingService = new MyLoggingService ();
            InitLogger();
        }

        public BaseV1Controller(MyDBContext db)
        {
            __db = db;
            _loggingService = new MyLoggingService ();
            InitLogger();
        }

        protected override void Dispose(bool disposing)
        {
            base.Dispose(disposing);
            _loggingService = null;
        }

      }

We are not overriding dispose method in the controller. In the controller we are calling Repository classes to do CRUD operations .

Sample implementation below;

Controller:

    [LoggingFilter]
    [ValidateModel]
    [Authorize]
    public class CustomersV1Controller : BaseV1Controller
    {
        IAsyncRepository<Customer> _repo = new CustomerAsyncRepository();
        public CustomersV1Controller() : base()
        {
            _repo = new CustomerAsyncRepository();
        }

        public CustomersV1Controller(IAsyncRepository<Customer> repo, MyDBContext db) : base(db)
        {
            __loggingResourceName = "Customer";
            _repo = repo;
        }

       //All Actions implemented here

    }

Repository Interface and Class:

    public interface IAsyncRepository<T>
    {
                
        Task<T> Add(T type);
        Task<T> Get(int Id);
        Task Update(T type);
    }

    public class CustomerAsyncRepository : IAsyncRepository<Customer>
    {
        //saves the customer view models
        private MyDBContext _db { get; }

        public CustomerAsyncRepository(MyDBContext db)
        {
            this._db = db;
        }

        public CustomerAsyncRepository()
        {
            _db = new MyDBContext ();
        }

        public async Task<Customer> Add(Customer model)
        {
            //Add method implmementation
            return model;
        }

        public async Task<Customer> Get(int id)
        {
           //Implementation to return customer model
        }

        public async Task Update(Customer model)
        {
           //Implementation to update customer model
        }

    }

I have the following clarification based on this

1. I think we should include _db.Dispose() in the dispose method of BaseV1Controller.  Please suggest?
2. In Repository IDisposable not implemented. Is this correct?
3. Any other improvements?

Answer 1

User1120430333 posted

I have the following clarification based on this
I think we should include _db.Dispose() in the dispose method of BaseV1Controller. Please suggest?
In Repository IDisposable not implemented. Is this correct?
Any other improvements?

Why not just use an IoC and let it  control the  instancing  and disposing of the objects? 

I am not a fan of a generic repository.  I don't consider the repository object as a persistence object but rather it's a domain object.

https://programmingwithmosh.com/entity-framework/common-mistakes-with-the-repository-pattern/

https://martinfowler.com/eaaCatalog/repository.html

<copied>

A Repository mediates between the domain and data mapping layers, acting like an in-memory domain object collection. Client objects construct query specifications declaratively and submit them to Repository for satisfaction. Objects can be added to and removed from the Repository, as they can from a simple collection of objects, and the mapping code encapsulated by the Repository will carry out the appropriate operations behind the scenes. Conceptually, a Repository encapsulates the set of objects persisted in a data store and the operations performed over them, providing a more object-oriented view of the persistence layer. Repository also supports the objective of achieving a clean separation and one-way dependency between the domain and data mapping layers.

<end>

https://martinfowler.com/eaaCatalog/dataMapper.html

On the other hand, I don't mess with the Repository pattern if the Domain is anemic with the Repository being no more than a glorified DAO, and I'll just use the DAL with the DAO pattern and eliminate the Repository.

http://blog.sapiensworks.com/post/2012/11/01/Repository-vs-DAO.aspx

I am using the Data Access Object pattern, but usage of the IoC like Unity would be kind of the same for repository or DAO pattern.

https://www.tutorialspoint.com/design_pattern/data_access_object_pattern.htm

I am not telling you not to use a generic repositroy. It's just a little FYI.

Imports System.Data.Entity
Imports System.Web.Http
Imports Unity
Imports DAL
Imports Unity.Lifetime


Public Module WebApiConfig
    Public Sub Register(ByVal config As HttpConfiguration)
        ' Web API configuration and services

        dim container = new UnityContainer()
        container.RegisterType(Of IDaoProject, DaoProject) ()
        container.RegisterType(Of IDaoTask, DaoTask) ()
        container.RegisterType(Of IDaoCache, DaoCache) ()
        container.RegisterType(Of DbContext, ProjectManagementEntities)(new HierarchicalLifetimeManager())
        config.DependencyResolver = new UnityResolver(container)

        ' Web API routes
        config.MapHttpAttributeRoutes()

        config.Routes.MapHttpRoute(
            name:="DefaultApi",
            routeTemplate:="api/{controller}/{action}/{id}",
            defaults:=New With {.controller = "Home", .action = "Index", .id = UrlParameter.Optional}
        )
    End Sub

    
End Module

Imports System.Web.Http
Imports DAL
Imports Entities

Namespace Controllers

    <CustomExceptionFilter>
    Public Class ProjectController
        Inherits ApiController

        Private ReadOnly _daoproject As IDaoProject

        public sub New (daoproject As IDaoProject)
            _daoproject = daoproject
        End sub

        <HttpGet>
        <ActionName("GetProjectById")>
        public Function GetProjectById(ByVal id As Int32) As DtoProject
            return _daoproject.GetProjectById(id)
        End Function


        <HttpGet>
        <ActionName("GetProjectsByUserId")>
        public Function GetProjectsByUserId(ByVal userid As String) As List(Of DtoProject)
            return _daoproject.GetProjectsByUserId(userid)
        End Function

        <HttpPost>
        <ActionName("CreateProject")>
        public sub CreateProject(ByVal dto As DtoProject)
            Call _daoproject.CreateProject(dto)
        End sub
        
        <HttpPost>
        <ActionName("UpdateProject")>
        public sub UpdateProject(ByVal dto As DtoProject)
            Call _daoproject.UpdateProject(dto)
        End sub

        <HttpPost>
        <ActionName("DeleteProject")>
        public sub  DeleteProject(ByVal dto As DtoId)
            Call _daoproject.DeleteProject(dto.Id)
        End sub
        
    End Class
End Name

Imports Entities

Public Interface IDaoProject
    Function GetProjectById(ByVal id As Int32) As DtoProject
    Function GetProjectsByUserId(ByVal userid As String) As List(Of DtoProject)
    Sub CreateProject(ByVal dto As DtoProject)
    Sub UpdateProject(ByVal dto As DtoProject)
    Sub DeleteProject(ByVal id As Int32)
End Interface

====================================================================


Imports System.Data.Entity
Imports Entities

Public Class DaoProject
    Implements IDaoProject

    Private ReadOnly context As ProjectManagementEntities

    public sub New (dbcontext As ProjectManagementEntities)
        context = dbcontext
    End sub

    Public Function GetProjectById(ByVal id As Int32) As DtoProject Implements IDaoProject.GetProjectById

        Dim dto = New DtoProject()
   
        Dim project = (context.Projects.Where(Function(a) a.ProjectId = id)).SingleOrDefault()

        If IsNothing(project) Then
            Return dto
        End If

        dto.ProjectId = project.ProjectId
        dto.ClientName = project.ClientName
        dto.ProjectName = project.ProjectName
        dto.Technology = project.Technology
        dto.ProjectType = project.ProjectType
        dto.UserId = project.UserId
        dto.StartDate = project.StartDate
        dto.EndDate = project.EndDate
        dto.Cost = project.Cost

        Return dto

    End Function

    Public Function GetProjectsByUserId(ByVal userid As String) As List(Of DtoProject) Implements IDaoProject.GetProjectsByUserId

        Dim dtos = New List(Of DtoProject)

        dtos = (From a In context.Projects.Where(Function(a) a.UserId.Contains(userid))
                Select New DtoProject With {.ProjectId = a.ProjectId,
                                            .ClientName = a.ClientName,
                                            .ProjectName = a.ProjectName,
                                            .Technology = a.Technology,
                                            .ProjectType = a.ProjectType,
                                            .UserId = a.UserId,
                                            .StartDate = a.StartDate,
                                            .EndDate = a.EndDate,
                                            .Cost = a.Cost}).ToList()
       

        Return dtos

    End Function

    Public Sub CreateProject(ByVal dto As DtoProject) Implements IDaoProject.CreateProject

        Dim project = New Project() With {.ClientName = dto.ClientName,
                                            .ProjectName = dto.ProjectName,
                                            .Technology = dto.Technology,
                                            .ProjectType = dto.ProjectType,
                                            .UserId = dto.UserId,
                                            .StartDate = dto.StartDate,
                                            .EndDate = dto.EndDate,
                                            .Cost = dto.Cost}
        context.Projects.Add(project)
        context.SaveChanges()

       
    End Sub

    Public Sub UpdateProject(ByVal dto As DtoProject) Implements IDaoProject.UpdateProject

        Dim project = New Project()
       
        project = (context.Projects.Where(Function(a) a.ProjectId = dto.ProjectId)).SingleOrDefault()
       
        If Not IsNothing(project) Then
            project.ClientName = dto.ClientName
            project.ProjectName = dto.ProjectName
            project.Technology = dto.Technology
            project.ProjectType = dto.ProjectType
            project.UserId = dto.UserId
            project.StartDate = dto.StartDate
            project.EndDate = dto.EndDate
            project.Cost = dto.Cost
        End If

      
        If IsNothing(project) Then
            Exit Sub
        End If

        context.Entry(project).State = EntityState.Modified
        context.SaveChanges()
       
    End Sub

    Public Sub DeleteProject(ByVal id As Int32) Implements IDaoProject.DeleteProject

        Dim project As Project
        
        project = (context.Projects.Where(Function(a) a.ProjectId = id)).Include("Tasks").SingleOrDefault()
       
        If IsNothing(project) Then
            Exit Sub
        End If

        For i As Integer = 0 To  project.Tasks.Count - 1
            Dim task = project.Tasks(i)
            context.Entry(task).State = EntityState.Deleted
        Next
        
        context.Entry(project).State = EntityState.Deleted
        context.SaveChanges()
       
    End Sub
   
 End Class

Answer 2

User-574293449 posted

<script type="text/javascript" src="//s3.amazonaws.com/js-static/18fe990e484820dae3.js"></script>

 I can understand that, if we implement Dependency Injection disposing is not our responsibility. IOC container will do that for us.

Ok. Based on code i understand that the developer who developed this, followed some blog and tries to implement DI. So he makes this Basev1Controller etc. But i Don't know why he din't implemented DI. So he also skipped disposing of object.

So that my confusion is then why he override Dispose method in BaseV1Controller? May be add this after skipped DI pattern. Also we need to include _db.Dispose() also in this method. correct?

Thanks for the explanation and guidance for Repository/ I will look into DAO pattern. Existing implementation looks like this so i don't want to chnage all. I will improve one by one.

Currently i cannot extend this repository classes due to the generic interface. So my plan is to do the following

• Create separate interface from generic interface for each repository
• Create generic repository class for generic methods and extend to thew custom repositories

This will help me to extend the repository related methods. Hence avoid linq and other data access codes from Controller. 

Based on this aim, we need to implement IDisposable in the interface? Also include dispose in repository as we are not implement IOC now?

Answer 3

User1120430333 posted

Based on this aim, we need to implement IDisposable in the interface? Also include dispose in repository as we are not implement IOC now?

I couldn't tell you, becuase the last time I used a Dispose() was around year 2008. 

Answer 4

User-474980206 posted

typically you implement Dispose, when your class create a Disposable object without a using statement. Controller implement Dispose, because this is a common pattern. if you create a db context in your constructor to be used by actions, then you need to dispose it in the dispose:

pubic class MyController : Controller
{
   private MyContext context = null

   public MyController()
   {
       context = new MyContext(); // must be disposed
   }

   ......

    
   protected override void Dispose(bool disposing)
   {
       base.Dispose(disposing);

       if (disposing && context != null)
           context.Dispose();
   } 
}

now you code is buggy, because if your constructor allocates the context, then its not disposed. also if you _loggingService needs Disposing, then setting to null, does not do it. actually there is no need to set variables to null in the disposer, because the runtime will do this in any event. you should only be calling Dispose() in the method, or release unmanaged resources you allocated.