0

I'd like to inject a number of interfaces to another service. Let's take a look at 2 services that I want to have their dependency injected.

Inside Term.cs

private readonly IWSConfig WSConfig;
private readonly IMemoryCache MemCache;

public Term(IWSConfig wsConfig, IMemoryCache memoryCache)
{
    WSConfig = wsConfig;
    MemCache = memoryCache;
}


public async Task LoadData()
{
    List<ConfigTerm> configTerm = await WSConfig.GetData(); // This is a web service call
    ...
}

Inside Person.cs

private readonly PersonRepo PersonRepository;
private readonly IMemoryCache MemCache;
private readonly ITerm Term;
private readonly IWSLoadLeave LoadLeave;
private readonly IWSLoadPartics LoadPartics;

public Person(PersonRepo personRepository, IMemoryCache memCache, ITerm term, IWSLoadLeave loadLeave, IWSLoadPartics loadPartics)
{
    PersonRepository = personRepository;
    MemCache = memCache;
    Term = term;
    LoadLeave = loadLeave;
    LoadPartics = loadPartics;
}

Code in Startup.cs

services.AddDbContext<DBContext>(opts => opts.UseOracle(RegistryReader.GetRegistryValue(RegHive.HKEY_LOCAL_MACHINE, Configuration["AppSettings:RegPath"], "DB.ConnectionString", RegWindowsBit.Win64)));
services.AddTransient<ILogging<ServiceLog>, ServiceLogRepo>();
services.AddSingleton<IMemoryCache, MemoryCache>();    
services.AddHttpClient<IWSConfig, WSConfig>();
services.AddHttpClient<IWSLoadLeave, WSLoadLeave>();
services.AddHttpClient<IWSLoadPartics, WSLoadPartics>();

var optionsBuilder = new DbContextOptionsBuilder<DBContext>(); // Can we omit this one and just use the one in AddDbContext?
optionsBuilder.UseOracle(RegistryReader.GetRegistryValue(RegHive.HKEY_LOCAL_MACHINE, Configuration["AppSettings:RegPath"], "DB.ConnectionString", RegWindowsBit.Win64));

services.AddSingleton<ITerm, Term>((ctx) => {
    WSConfig wsConfig = new WSConfig(new System.Net.Http.HttpClient(), new ServiceLogRepo(new DBContext(optionsBuilder.Options))); // Can we change this to the IWSConfig and the ILogging<ServiceLog>
    IMemoryCache memoryCache = ctx.GetService<IMemoryCache>();
    return new Term(wsConfig, memoryCache);
});
services.AddSingleton<IPerson, Person>((ctx) => {
    PersonRepo personRepo = new PersonRepo(new DBContext(optionsBuilder.Options)); // Can we change this?
    IMemoryCache memoryCache = ctx.GetService<IMemoryCache>();
    ITerm term = ctx.GetService<ITerm>();
    WSLoadLeave loadLeave = new WSLoadLeave(new System.Net.Http.HttpClient(), new ServiceLogRepo(new DBContext(optionsBuilder.Options))); // Can we change this?
    WSLoadPartics loadPartics = new WSLoadPartics(new System.Net.Http.HttpClient(), new ServiceLogRepo(new DBContext(optionsBuilder.Options))); // Can we change this?
    return new Person(personRepo, memoryCache, term, loadLeave, loadPartics);
});

But there are some duplication here and there. I've marked as the comments in the code above. How to correct it ?

[UPDATE 1]: If I change the declaration from singleton with the following:

services.AddScoped<ITerm, Term>();
services.AddScoped<IPerson, Person>();

I'm getting the following error when trying to insert a record using the DbContext.

{System.ObjectDisposedException: Cannot access a disposed object. A common cause of this error is disposing a context that was resolved from dependency injection and then later trying to use the same context instance elsewhere in your application. This may occur if you are calling Dispose() on the context, or wrapping the context in a using statement. If you are using dependency injection, you should let the dependency injection container take care of disposing context instances. Object name: 'DBContext'.

In my WSConfig, it will inherit a base class. This base class also have reference to the ServiceLogRepo, which will call the DbContext to insert a record to the database

In WSConfig

public class WSConfig : WSBase, IWSConfig
{
    private HttpClient WSHttpClient;

    public WSConfig(HttpClient httpClient, ILogging<ServiceLog> serviceLog) : base(serviceLog)
    {
        WSHttpClient = httpClient;

        //...
    }

    //...
}

The WSBase class:

public class WSBase : WSCall
{    
    private readonly ILogging<ServiceLog> ServiceLog;

    public WSBase(ILogging<ServiceLog> serviceLog) : base(serviceLog)
    {

    }

    ...
}

The WSCall class:

public class WSCall 
{
    private readonly ILogging<ServiceLog> ServiceLog;

    public WSCall(ILogging<ServiceLog> serviceLog)
    {
        ServiceLog = serviceLog;
    }
    ....
}

And the ServiceLogRepo code

public class ServiceLogRepo : ILogging<ServiceLog>
{
    private readonly DBContext _context;

    public ServiceLogRepo(DBContext context)
    {
        _context = context;
    }

    public async Task<bool> LogRequest(ServiceLog apiLogItem)
    {
        await _context.ServiceLogs.AddAsync(apiLogItem);
        int i = await _context.SaveChangesAsync();

        return await Task.Run(() => true);
    }
}

I also have the following in Startup.cs to do the web service call upon application load.

public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory, ITerm term)
{
    ....
    System.Threading.Tasks.Task.Run(async () => await term.LoadData());
}

It seems when going into term.LoadData(), the DBContext is disposed already.

rcs
  • 6,713
  • 12
  • 53
  • 75
  • Why not just replace your `new DBContext()` with `ctx.GetService< NSWSDBContext >()`? – zaitsman Nov 05 '19 at 02:27
  • Did that, but it gets `Cannot resolve scoped service 'Prj.Entity.DBContext' from root provider.'`. Btw I've fixed the typo, the type name is DBContext. – rcs Nov 05 '19 at 02:36
  • Is there a reason you don't simply register `ITerm` like so?: `services.AddSingleton()` - Assuming `Term` accepts an `IWSConfig`, it should just resolve it from the container. This seems very similar to your last question, and seems to have the same solution. – ProgrammingLlama Nov 05 '19 at 02:37
  • @John I get the following error if I change like that: `InvalidOperationException: Cannot resolve 'Prj.Log.Interfaces.ILogging`1[Prj.Entity.ServiceLog]' from root provider because it requires scoped service 'Prj.Entity.DBContext'.` ... `Exception: Could not resolve a service of type 'Prj.WebAPI.Helpers.Interfaces.ITerm' for the parameter 'term' of method 'Configure' on type 'Prj.Startup'.` ... – rcs Nov 05 '19 at 02:40
  • 1
    So your problem is that you registered DB Context scoped in a web app, and then you're trying to inject it as singleton. a) of course it won't work b) instead of injecting the db context to singleton, inject a factory. resolve a singleton in the operation and dispose at the end. Otherwise it is likely you will have a db connection for the lifetime of the app. – zaitsman Nov 05 '19 at 02:42
  • 1
    Based on singleton DbContext being bad (according to [this question](https://stackoverflow.com/questions/29153748/singleton-scope-for-efs-dbcontext)), and not being able to use a scoped service in a singleton, I'd suggest rethinking your approach. Do `Term` and `Person` need to be singleton? – ProgrammingLlama Nov 05 '19 at 02:44
  • I used singleton for ITerm and IPerson, due to I want to have only one httpclient for the lifetime of the app. If I create using scoped, then it will create multiple httpclients. HttpClient is more suitable to be used in singleton. – rcs Nov 05 '19 at 02:50
  • Forget `HttpClient`. The whole HttpClientFactory stuff (which you're using!) was designed to deal with the old issues. Multiple `HttpClient` instances are not the problem, but multiple `HttpClientHandler` instances _are_. HttpClientFactory handles these for you. Read [this](https://aspnetmonsters.com/2016/08/2016-08-27-httpclientwrong/) and [this](https://josefottosson.se/you-are-probably-still-using-httpclient-wrong-and-it-is-destabilizing-your-software/) (which covers both why singleton HttpClient is bad, and how HttpClientFactory helps) – ProgrammingLlama Nov 05 '19 at 02:52
  • 1
    @rcs if using AddHttpClient then that is being managed for you already – Nkosi Nov 05 '19 at 02:52
  • I've updated my question, but I encounter another error when trying to use the ServiceLog to insert a record to the database. – rcs Nov 05 '19 at 03:14
  • @Nkosi, the WSCall is being inherited by WSConfig, I've updated the code. – rcs Nov 05 '19 at 03:36
  • Did you follow the instruction of the exception message about how to deal with DbContext? – Nkosi Nov 05 '19 at 03:38
  • @Nkosi There is no Dispose call. I wonder how it gets disposed. If you see, when I try to run `System.Threading.Tasks.Task.Run(async () => await term.LoadData());`. The DbContext is disposed already. – rcs Nov 05 '19 at 03:42
  • 1
    Because it is not awaited, it is being disposed before you are finished with the db context – Nkosi Nov 05 '19 at 03:43
  • Where does the term used in startup come from? – Nkosi Nov 05 '19 at 03:43
  • @Nkosi, the function is called in the `Configure` function, which will be called after all the declaration of services completed. – rcs Nov 05 '19 at 03:46
  • 2
    Don't use `Task.Run` like you are. Even if this was in the context of a web request, you can't run code in another task (fire and forget) and expect it to work like that. You and a friend walk into a shop. You given them a hammer so they can hit some nails in the floor of the shop. Its a funny hammer though - it explodes when you leave the shop (it is _scoped_ to your presence in the shop). What do you think will happen if they stay in the shop and start hammering nails in **and you leave without them**? – mjwills Nov 05 '19 at 03:49
  • Ur Person class takes Personrepository as parameter. This is very bad design. Ur person repo should be taking person as parameter instead. – Gauravsa Nov 05 '19 at 03:49
  • Can you talk us through why you want to call `term.LoadData` from `Configure` / `Startup`? – mjwills Nov 05 '19 at 03:52
  • @mjwills Because I want to pre-load some data from a web service call. The call may take some time, so when the app starts, I want it running in the background, so when user comes in, they will see the data is already there. – rcs Nov 05 '19 at 03:53

1 Answers1

1

First properly register all the necessary dependencies in ConfigureServices using the appropriate liftetime scopes

services.AddDbContext<DBContext>(opts => opts.UseOracle(RegistryReader.GetRegistryValue(RegHive.HKEY_LOCAL_MACHINE, Configuration["AppSettings:RegPath"], "DB.ConnectionString", RegWindowsBit.Win64)));
services.AddTransient<ILogging<ServiceLog>, ServiceLogRepo>();
services.AddSingleton<IMemoryCache, MemoryCache>();    
services.AddHttpClient<IWSConfig, WSConfig>();
services.AddHttpClient<IWSLoadLeave, WSLoadLeave>();
services.AddHttpClient<IWSLoadPartics, WSLoadPartics>();

services.AddScoped<ITerm, Term>();
services.AddScoped<IPerson, Person>();

Given the async nature of the method being called in Configure the DbContext is being disposed before you are done with it.

Now ideally given what you are trying to achieve you should be using a background service IHostedServive which will be started upon startup of the application.

public class TermHostedService : BackgroundService {
    private readonly ILogger<TermHostedService> _logger;

    public TermHostedService(IServiceProvider services, 
        ILogger<ConsumeScopedServiceHostedService> logger) {
        Services = services;
        _logger = logger;
    }

    public IServiceProvider Services { get; }

    protected override async Task ExecuteAsync(CancellationToken stoppingToken) {
        _logger.LogInformation("Term Hosted Service running.");

        using (var scope = Services.CreateScope()) {
            var term = scope.ServiceProvider.GetRequiredService<ITerm>();    
            await term.LoadData();
            _logger.LogInformation("Data Loaded.");
        }
    }

    public override async Task StopAsync(CancellationToken stoppingToken) {
        _logger.LogInformation("Term Hosted Service is stopping.");    
        await Task.CompletedTask;
    }
}

when registered at startup

services.AddHostedService<TermHostedService>();

Reference Background tasks with hosted services in ASP.NET Core

Nkosi
  • 235,767
  • 35
  • 427
  • 472