The ServiceClient class you refer to implements IDisposible, so it should be disposed explicitly once you are done with it, otherwise you may end up with memory leaks or other issues. The issue you have here is that you are scoping CrmRepository, and not scoping ServiceClient, therefore not explicitly managing the ServiceClient's lifetime.
A "scoped" service exists in memory for as long as the scope lasts for. This is different depending on the context of the application. To answer accurately, we would need to know if this is a WebAPI app, an MVC app, or a Blazor app etc.
Generally speaking you should be registering as many dependencies as possible using your IoC container so you can manage the scope of every dependency independently. However, in this instance - purely because the ServiceClient class is a black box, I would simply treat the CrmRepository as a grey box, remove the ServiceClient dependency (which I can see has no further abstractions from the documentation - making it generally difficult to mock, and therefore making registering it as a dependency relatively pointless), and write integration tests for the CrmRepository itself.
Your code would therefore end up as:
Registration:
services.Configure<CrmRepoOptions>("path:to:your:connectionString");
services.AddScoped<ICrmRepository, CrmRepository>();
Implementation:
public class CrmRepoOptions
{
public string ConnectionString { get; set; }
}
public class CrmRepository: ICrmRepository
{
private readonly CrmRepoOptions _opt;
public CrmRepository(IOptions<CrmRepoOptions> opt)
{
_opt = opt.Value;
}
public Task DoSomethingWithServiceClient()
{
using(var serviceClient = new ServiceClient(_opt.ConnectionString))
{
// Do your magic with serviceClient
}
return Task.CompletedTask;
}
}
It is unlikely you would want the ServiceClient instance to exist for the entire scope of the CrmRepository (I expect you only need it when DoSomethingWithServiceClient is called), but if you did want it to exist for as long as the CrmRepository for a particular reason, you would be better off implementing IDisposible in the CrmRepository, creating the ServiceClient in the CrmRepository's constructor, and dispose of it in the Dispose method:
public class CrmRepository: IDisposable, ICrmRepository
{
private readonly CrmRepoOptions _opt;
private Serviceclient _sc;
public CrmRepository(IOptions<CrmRepoOptions> opt)
{
_opt = opt.Value;
_sc = new ServiceClient(_opt.ConnectionString);
}
public Task DoSomethingWithServiceClient()
{
// Do your magic with _sc
return Task.CompletedTask;
}
/// <inheritdoc />
public void Dispose()
{
// Not production ready - implement a proper dispose pattern here
_sc.Dispose();
}
}
Have a think about how you want to use ServiceClient and if you need a new instance of it every time you call your method or not. The documentation likely has details around how it manages its internal connections. It may be that a scoped or singleton instance is a really bad idea, and the example code is the recommended way to use it.
-- EDIT --
As comments here have eluded to, you can also register the ServiceClient as a scoped class in your IoC container, meaning you would not have to instantiate it directly when you register the CrmRepository. Whilst this is achievable, the issue you will have here is that if the ServiceClient is injected into other classes (transient or scoped) that are created as part of the same WebAPI request, then the same ServiceClient instance will be used there too.
I do not believe you want to do this here; I believe that the first code example in my response above is actually what you're looking to do; create the ServiceClient whenever you need it, then dispose of it immediately once you have finished working with it. As with many "connection" type classes provided by MS, it is very likely some kind of connection management and / or pooling is happening in the background, and creating and maintaining a persistent (either singleton or scoped) connection will cause you issues in the future. What you are trying to do is likely an over-optimisation in this instance.
