0

I have created my data provider using Repository Pattern.

First, I designed a base repository interface as following:

internal interface IGenericRepository<T, in TResourceIdentifier>
{
    Task<IEnumerable<T>> GetManyAsync();
    Task<T> GetAsync(TResourceIdentifier id);
    Task PutAsync(T model);
    Task<T> PostAsync(T model);
    Task DeleteAsync(TResourceIdentifier id);
}

Then I implemented it:

public class GenericRepository<T, TResourceIdentifier> : IDisposable, IGenericRepository<T, TResourceIdentifier> 
    where T : class
{
    private bool _disposed;
    protected HttpClientHelper<T, TResourceIdentifier> Client;

    protected GenericRepository(string addressSuffix)
    {
        Client = new HttpClientHelper<T, TResourceIdentifier>(Properties.Settings.Url, addressSuffix);
    }

    public async Task<IEnumerable<T>> GetManyAsync()
    {
        return await Client.GetManyAsync();
    }

    // All other CRUD methods and dispose

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }

    private void Dispose(bool disposing)
    {
        if(_disposed || !disposing) return;

        if(Client != null)
        {
            var mc = Client;
            Client = null;
            mc.Dispose();
        }

        _disposed = true;
    }
}

Then I created custom repository interface for each of my entities. For example:

internal interface IOrderRepository : IGenericRepository<Order, int>
{
    Task<IEnumerable<Order>> GetOrderBySomeConditionAsync(string condition );

}

And finally, I implemented the custom repository:

public class OrderRepository : GenericRepository<Order, int>, IOrderRepository
{
    public OrderRepository(string addressSuffix) : base(addressSuffix)
    {
    }

    public async Task<IEnumerable<Order>> GetOrderBySomeConditionAsync(string condition)
    {
        //get all the orders (GetManyAsync()) and then returns the ones meeting the condition
    }
}

Note that HttpClientHelperuses HttpClient and needs to be disposed manually.

I have created a MVC web application and have defined the repositories at the class level as such:

IOrderRepository _orderRepository = new OrderRepository();

When I call _orderRepository in my CRUD operations, it does not hit dispose after its use. In order to fix that I have ended up implementing like this:

private async Task<IEnumerable<OrderViewModel>> GetOrders()
{
    using(var orderRepository = new OrderRepository())
         return await orderRepository.GetManyAsync();
}

This would hit the Dispose but is anti pattern.

What am I missing in my implementation that the instance is not disposed on each call?

amindomeniko
  • 417
  • 6
  • 23
  • Why **should** it dispose on each call? – A Friend Sep 20 '19 at 15:10
  • @AFriend When should it be disposed then? – amindomeniko Sep 20 '19 at 15:11
  • @amindomeniko refer to https://stackoverflow.com/questions/2871888/dispose-when-is-it-called – SpiritBob Sep 20 '19 at 15:13
  • 1
    It should be disposed once the controller is no longer required. If you continue with the current code then you are likely to run into sockets issues as explained in one of the given answers, as well as additional overhead of recreating the repository on every single request. – A Friend Sep 20 '19 at 15:24
  • @AFriend ok, I read the provided articles and posts provided by friends on here. So having an MVC web application, the pages and the controllers consequently could be called back and forth multiple times. Where would I dispose the dataprovider? When session is ended? Any example please? – amindomeniko Sep 20 '19 at 17:17

2 Answers2

3

You should not be disposing HTTPClient after every request.

[https://learn.microsoft.com/en-us/dotnet/architecture/microservices/implement-resilient-applications/use-httpclientfactory-to-implement-resilient-http-requests][1]

As the above link says -

Therefore, HttpClient is intended to be instantiated once and reused throughout the life of an application. Instantiating an HttpClient class for every request will exhaust the number of sockets available under heavy loads. That issue will result in SocketException errors. Possible approaches to solve that problem are based on the creation of the HttpClient object as singleton or static, as explained in this Microsoft article on HttpClient usage.

R Jain
  • 486
  • 3
  • 9
  • Thanks for the article. Now I understand why instantiating HttpClient on every call is not a good idea. So given I am using my repositories in an MVC web application where there could be multiple calls back and forth on the controllers using the repositories, how should I dispose my instance? When the user signs out of the application? – amindomeniko Sep 20 '19 at 17:25
1

Writing a Dispose method in your generic repository does not mean it will be invoked automatically, whenever you feel like it should. It's intended to be invoked individually, hence why you must either use a using statement (as you have shown), or the Dispose method inside your code.

Alternatively, you can leave that job for the garbage collector.

You should also create a finalizer in your generic repository, if you're convinced on using GC.SuppressFinalize(this);

Read more about that here - When should I use GC.SuppressFinalize()?

As R Jain pointed out, you should also create a static class to hold your HttpClient. You'll have to use HttpResponseMessages for your needs, or HttpContent.

Some more resources to read:

  1. HttpClient single instance with different authentication headers
  2. https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/
SpiritBob
  • 2,355
  • 3
  • 24
  • 62
  • thanks for the information. I have implemented my HttpClient class separately under HttpClientHelper as you can see my code snippet. What I need to figure now is to when to dispose the repository instance given i am instantiating them in a controller class and the pages and controllers can be called multiple times back and forth. Should I dispose when user signs out? What if there is no user sign in for an application? – amindomeniko Sep 20 '19 at 17:26
  • @amindomeniko you are not disposing the repository instance if that's what you want to dispose. You're disposing the `HttpClient` inside that repository class. You should never do that as previously pointed out. Use a single static `HttpClient` instance, and pass all your requests through it. The repository instance is influenced by the scope at which you used to implement the service for dependency injection. (scoped/singleton, not sure transient exists for ASP.NET MVC) If it's scoped - a new instance of the repository is injected for every request. Singleton is one such instance for all. – SpiritBob Nov 26 '19 at 16:44