locked
Disposing Objects in WebAPI RRS feed

  • 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?
    Monday, September 17, 2018 9:54 AM

Answers

  • 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

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Monday, September 17, 2018 11:01 AM
  • 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. 


     

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Wednesday, September 26, 2018 3:48 PM

All replies

  • 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

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Monday, September 17, 2018 11:01 AM
  • User-574293449 posted

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

    cool 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?

    Tuesday, September 18, 2018 1:49 AM
  • 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. 

    Tuesday, September 18, 2018 2:58 PM
  • 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. 


     

    • Marked as answer by Anonymous Thursday, October 7, 2021 12:00 AM
    Wednesday, September 26, 2018 3:48 PM