ipjohnson / Grace.DependencyInjection.Extensions

Grace Extensions for ASP.Net Core
19 stars 7 forks source link

Support for Generic Host based Worker Service #19

Open brian-imaginelearning opened 4 years ago

brian-imaginelearning commented 4 years ago

Although I could be missing something, I don't see support for a Generic Host (non-web) based app, such as a fresh project using the new Worker Service template in VS 2019.

I can call UseGrace on IHostBuilder, but since there is no Startup, there doesn't appear to be a means for container registrations. For example, Stashbox.Extension.Hosting does it like this:

using (var host = new HostBuilder()
    .UseStashbox()
    .ConfigureContainer<IStashboxContainer>((context, container) =>
    {
        container.Register<Foo>();
    })
    .ConfigureServices((context, services) =>
    {
        services.AddHostedService<Service>();
    })
    .Build())
{
    // start and use your host
}

Alternatively, it could be done similar to LightInject (with the disadvantage of not having access to HostBuilderContext which is particularly useful for access to IConfiguration)

var host = new HostBuilder().UseLightInject(r => r.Register<IBar, Bar>()).Build();
var foo = host.Services.GetRequiredService<IFoo>();

Ideally this would be in a separate nuget package, to minimize MS dependencies to only Microsoft.Extensions.Hosting.Abstractions.

Thanks, would love to investigate adopting Grace for a project, as it looks promising, but need to resolve this first.

ipjohnson commented 4 years ago

That's a good suggestion. Let me look at putting together a new package that's dependent on the only the hosting abstraction and call it Grace.Extensions.Hosting

ipjohnson commented 4 years ago

I've checked in a new project, would you be able to try it from the nightly nuget feed?

https://ci.appveyor.com/nuget/grace-dependencyinjection-extensions-nightly

using (var hosting = new HostBuilder().UseGrace().ConfigureContainer<IInjectionScope>((context, scope) =>
{
      scope.Configure(c => c.Export<Test>().Lifestyle.Singleton());
}).Build())
{
}
brian-imaginelearning commented 4 years ago

Wow, that was fast! First blush take porting an existing working sample is a problem in binding options to configuration with data annotation validation. That could be relatated, a separate issue, or my bug. :) I'll work on minimizing repro to a minimal sample and will get with you soon.

brian-imaginelearning commented 4 years ago

Ok, here is a minimal repro, using very simple binding of configuration to an options class. It works as written, but fails if un-commenting the code that uses Grace.

    public class Program
    {
        public static void Main(string[] args)
        {
            Environment.SetEnvironmentVariable("Name", "TestWorkerName");

            Host.CreateDefaultBuilder(args)
                //.UseGrace()
                //.ConfigureContainer<IInjectionScope>((context, scope) =>
                //{
                //})
                .ConfigureServices((hostContext, services) =>
                {
                    services.AddOptions<WorkerOptions>().Bind(hostContext.Configuration);
                    services.AddHostedService<Worker>();
                })
            .Build().Run();
        }
    }

    public class Worker : BackgroundService
    {
        private readonly ILogger _logger;

        public Worker(ILogger<Worker> logger, IOptions<WorkerOptions> options)
        {
            _logger = logger;
            var name = options.Value.Name;

            if (name == null)
                throw new Exception("Binding WorkerOptions failed");
            _logger.LogInformation("Worker name is " + name);
        }

        protected override async Task ExecuteAsync(CancellationToken stoppingToken)
        {
            while (!stoppingToken.IsCancellationRequested)
            {
                _logger.LogInformation("Worker running at: {time}", DateTimeOffset.Now);
                await Task.Delay(1000, stoppingToken);
            }
        }
    }

    public class WorkerOptions
    {
        public string Name { get; set; }
    }
brian-imaginelearning commented 4 years ago

BTW, similar binding works using Grace.AspNetCore.Hosting in a typical web app, so it must be something related to this new integration.

P.S. You might already know this, but when I first accidentally tried the latest version of Grace.AspNetCore.Hosting from the nightly repro, that failed even starting up from brand-new project from built-in VS template and just adding UseGrace. When I switched to released version, all was well.

ipjohnson commented 4 years ago

Looks like this wasn't related to the hosting only change rather it was something that was introduced in the most recent beta that just went out. Essentially the change was to introduce the named options as keyed exports. Problem was that all options were being exported as keyed not just the named.

I've tested your sample to confirm it works. If you could give it a whirl and see if you find any other issues before I push an official package to NuGet

brian-imaginelearning commented 4 years ago

Hmm, great, that works, but now binding as named options doesn't. Here is a slightly modified sample of the one above which works without Grace:

    public class Program
    {
        public static void Main(string[] args)
        {
            Environment.SetEnvironmentVariable("Worker:Name", "TestWorkerName");

            Host.CreateDefaultBuilder(args)
                //.UseGrace()
                //.ConfigureContainer<IInjectionScope>((context, scope) =>
                //{
                //})
                .ConfigureServices((hostContext, services) =>
                {
                    services.AddOptions<WorkerOptions>(nameof(Worker)).Bind(hostContext.Configuration.GetSection(nameof(Worker)));
                    services.AddHostedService<Worker>();
                })
            .Build().Run();
        }
    }

    public class Worker : BackgroundService
    {
        private readonly ILogger _logger;

        public Worker(ILogger<Worker> logger, IOptionsMonitor<WorkerOptions> options)
        {
            _logger = logger;
            var name = options.Get(nameof(Worker)).Name;

            if (name == null)
                throw new Exception("Binding WorkerOptions failed");
            _logger.LogInformation("Worker name is " + name);
        }

        protected override async Task ExecuteAsync(CancellationToken stoppingToken)
        {
            while (!stoppingToken.IsCancellationRequested)
            {
                _logger.LogInformation("Worker running at: {time}", DateTimeOffset.Now);
                await Task.Delay(1000, stoppingToken);
            }
        }
    }

    public class WorkerOptions
    {
        public string Name { get; set; }
    }
ipjohnson commented 4 years ago

I apologize I normally work with just the standard non-named options so I'd noticed this doesn't look to work.

I'll spend a little time today looking at this and understand the interaction between the container and configuration options.

ipjohnson commented 4 years ago

Sorry about all this ultimately looking at it the container really shouldn't be registering anything as keyed since the MS DI container doesn't support Keyed registration so I've rolled it back and everything should be working correctly now.

brian-imaginelearning commented 4 years ago

No worries. My sample project is working fine now. Seems ready for official package.

Thanks for implementing my suggestion! Looking forward to using Grace on new project.

ipjohnson commented 4 years ago

I haven't forgotten about this just been delayed on finding a solution for the PR that I'm having to roll back.

I believe I should have some time to get this released officially this weekend as I want to also bump the min version of Grace.