New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add instructions for using SimpleInjector with ASP.NET Core 2.0 IHostedService #633

Open
greyseal96 opened this Issue Nov 5, 2018 · 4 comments

Comments

2 participants
@greyseal96

greyseal96 commented Nov 5, 2018

Steven has a great answer to a question on StackOverflow about how to implement an IHostedService adapter and register it with Simple Injector and ASP.NET Core's service container. I think that it would be really helpful to add this to the documentation. I'd be willing to do this too. Just tell me where (which section) I should add it and I can create a documentation page and do a pull request.

@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented Nov 5, 2018

Feel free to cook up PR for this. The most obvious page to add this information to is the aspnetintegration.rst. You can use the 4.0 branch.

@greyseal96

This comment has been minimized.

greyseal96 commented Nov 17, 2018

To prep for making the addition, I was just reading through the aspnetintegration page again and I saw the UseMiddleware<T> extension method and that gave me an idea that I wanted to run past you...

I was looking at the source code for the AddHostedService<T> extension method and it is really simple. It just adds the hosted service to the IServiceCollection like so: services.AddTransient<IHostedService, THostedService>();

That made me wonder if it would be possible to create an extension method similar to what is being done with UseMiddleware<T>. But, since I'm not super familiar with the InstanceProducer class, I don't know if this would be a good idea or not. Just as a rough idea, I'm talking about an extension method sort of like this:

public static class HostedServiceExtensions
{
    public static IServiceCollection AddHostedServiceFromContainer<THostedService>(this IServiceCollection services, SimpleInjector.Container container)
        where THostedService : class, IHostedService
    {
        //The AddHostedService<T> extension method adds the services as transient so this would
        //be consistent with that.
        var lifestyle = SimpleInjector.Lifestyle.Transient;
        //Or would this be better?  It's similar to what the UseMiddleware extension method is doing...
        //var lifestyle = container.Options.LifestyleSelectionBehavior.SelectLifestyle(typeof(THostedService));

        SimpleInjector.InstanceProducer<THostedService> producer = lifestyle.CreateProducer<THostedService>(typeof(THostedService), container); 

        return services.AddTransient<IHostedService, THostedService>(svcProvider => producer.GetInstance());
    }
}

Would something like this work?

Also, I had a question about the recommendation from the SO question. In other things that you've written I've like how you've stressed the importance of making sure that classes clearly state their dependencies in their constructors. But, just having the hosted service implementation take a dependency on the Container seems like an exception to this rule; kind of like a service locator sort of thing. Is the reasoning behind this because it's something that we're forced to do because of Microsoft's decision to go with their design of the conforming container? Just curious. I've learned a lot about DI by using Simple Injector and reading what you and Mark Seemann have written so I was just wondering for my own personal knowledge.

@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented Nov 18, 2018

Your extension method is pretty good. It does take a different approach than my Stack Overflow answer, but the two are not mutually exclusive.

The SO answer pushes towards writing application-specific abstractions (e.g. IMyJob) and implementing the SimpleInjectorJobProcessorHostedService as an adapter. Your extension method instead lets hosted services implement IHostedService. That's not bad per see, and you could even register the SimpleInjectorJobProcessorHostedService using that extension method.

Do be warned, however, about your extension method. Even though the AddHostedService<T> extension method of ASP.NET Core, registers the services as Transient, hosted services are in fact singletons!! That's because they are resolved once from the framework and their StartAsync is called once soon after started, and StopAsync is called once at shutdown.

What this means is that while the hosted service can start a timer that runs periodically, that code will always use the same dependencies that were injected into the constructor. In other words, it would be an error to inject scoped (or transient) dependencies into the constructor of the hosted service (but ASP.NET Core doesn't protect you against this at all).

In other words, you should always register your hosted services as Singleton. This way Simple Injector can protect you from accidental Lifestyle Mismatches (a.k.a. Captive Dependencies).

So, I would suggest changing the extension method to the following:

public static IServiceCollection AddHostedServiceFromContainer<THostedService>(
    this IServiceCollection services, Container container)
    where THostedService : class, IHostedService
{
    var producer = Lifestyle.Singleton.CreateProducer<THostedService, THostedService>(container); 
    return services.AddSingleton<IHostedService, THostedService>(c => producer.GetInstance());
}

I thought about changing the CreateProducer call to the following:

var producer = Lifestyle.Singleton.CreateProducer<IHostedService, THostedService>(container); 

Advantage of such change is that it would allow hosted services to be decorated easily by registering a decorator (e.g. RegisterDecorator<IHostedService, LoggingHostedServiceDecorator>). However, this can cause problems when the decorator is registered as Transient or Scoped, because Simple Injector would be unable to detect that a Transient Decorator (with its dependencies) would be accidentally kept alive by ASP.NET Core.

Alternatively to the previous, the following code would have an identical effect:

container.RegisterSingleton<THostedService>();
return services.AddSingleton<IHostedService, THostedService>(
    c => container.GetInstance<THostedService>());
@dotnetjunkie

This comment has been minimized.

Collaborator

dotnetjunkie commented Nov 18, 2018

In other things that you've written I've like how you've stressed the importance of making sure that classes clearly state their dependencies in their constructors. But, just having the hosted service implementation take a dependency on the Container seems like an exception to this rule; kind of like a service locator sort of thing. Is the reasoning behind this because it's something that we're forced to do because of Microsoft's decision to go with their design of the conforming container?

In general, you don’t want to depend on the Container. That is absolutely correct. This has to do with the risk of it becoming a Service Locator, which is, as you understand, an anti-pattern. However, it is important to realize that not every usage of a Container is an implementation of the Service Locator anti-pattern.

What we are trying to achieve with dependency injection is loose coupling. This means that a class declares its required dependencies and pushes the responsibility of creating that dependencies up into the Composition Root. Result is that a class can become loosely coupled. The Composition Root, however, will always be tightly coupled. It knows about all classes in the system and if you use a DI Container, the Composition Root knows about the DI Container.

This is why Mark and I state in our book (also see Mark’s blog post about this subject) that the use of a Container inside the Composition Root is not a Service Locator. In the book we state:

Because the Composition Root already depends on everything else in the system […], it's impossible for it to drag along [the Container as] an extra Dependency. By definition, it knows about every Dependency already. And it's impossible for the Composition Root to hide its Dependencies—from whom does it hide them? Its role is to build object graphs; it doesn't need to expose those Dependencies.

And this is why the SimpleInjectorJobProcessorHostedService can take a dependency on the Container. It’s because that class is part of the Composition Root. Or at least, when you let a class depend on the Container (or an abstraction there over), you’d better place that class inside the Composition Root.

So this ‘exception’ has nothing to do with how Microsoft designed their system. The same thing would happen when you create an IControllerActivator implementation. It will wrap a Container to be able to resolve instances. Only difference is that we decided to create an implementation on your behalf, which is why you don’t have to implement it. The SimpleInjectorControllerActivator is an adapter, just like the SimpleInjectorJobProcessorHostedService. Since we will never implement all possible adapters for you, you will always have some adapters in your system, and when they depend on the Container, the right location to put them is your Composition Root. It has been this way with all frameworks that came before ASP.NET Core, and it will be this way with all frameworks them will come after ASP.NET Core.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment