simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 153 forks source link

What is the correct way to resolve a per-request service in a property-injection initializer block? #18

Closed jlew-cs closed 9 years ago

jlew-cs commented 9 years ago

I am using property injection in a few places in an ASP.NET MVC application where I am retrofitting SimpleInjector to replace Ninject. My question is: If I have properties that want to be injected with values that are have a "per-request" lifestyle, what is the best way to resolve them? Currently, my code looks like this:

container.RegisterInitializer<CerberusAuthorizeAttribute>(c =>
            {              
                c.AuthorizationModule = DependencyResolver.Current.GetService<ICerberusAuthorizationModule>();
                c.Order = 1;
            });

I'm using MVC's dependency resolver here, because it is aware of the current request scope, whereas I don't think(?) that the container would be. Is there another way to do this, that doesn't involve the MVC plumbing?

Thanks!

dotnetjunkie commented 9 years ago

There are multiple ways to do property injection in MVC attributes with Simple Injector. If you are using the RegisterMvcIntegratedFilterProvider you can either use the RegisterInitializer, as you are currently using. Besides calling into MVC's DependencyResolver, it's much cleaner to call back into the Container directly:

container.RegisterInitializer<CerberusAuthorizeAttribute>(attribute =>
{              
    attribute.AuthorizationModule = container.GetInstance<ICerberusAuthorizationModule>();
});

Another option is to configure a custom property selection behavior as described here. When added your custom property selection behavior in the container, you can mark your properties with any self-defined attribute. Note that Simple Injector has no built-in attribute for this as explained here.

In my opinion however, it is much better do stay away from dependency injection into attributes completely. I wrote why you should, here. This article describes an alternative that uses passive attributes.

An alternative to the passive attributes is to make the attribute a humble object. This means that your attribute contains as little code as possible and during execution only resolve a service that contains all logic to execute. This could look as follows:

public class CerberusAuthorizeAttribute : AuthorizeAttribute {
    public override void OnAuthorization(AuthorizationContext filterContext) {
        var service = DependencyResolver.Current.GetService<IAuthorizationService>();
        service.Authorize(filterContext);
    }
}

Do note that an IAuthorizationService is resolved. The implementation of this abstraction will get the ICerberusAuthorizationModule (and possible other dependencies) injected.

My preference is to go with the humble object if you have just one or two attributes, and go with passive attributes if you have more attributes -or- have attributes that can't derive from MVC's base attribute.

jlew-cs commented 9 years ago

Thanks for the detailed answer, I understand and agree with your preferences here. My main reason for asking the question is that I am fuzzy on what would happen specifically in this situation:

container.RegisterInitializer<CerberusAuthorizeAttribute>(c =>
{              
     c.AuthorizationModule = container.GetService<ICerberusAuthorizationModule>();
     c.Order = 1;
});

If ICerberusAuthorizationModule was registered per web request, will container.GetService grab an instance out of the current request context? If this was Autofac, for example, and container was the root container, it wouldn't do the right thing. I'm not sure if/how SI differs in this regard.

Thanks!

dotnetjunkie commented 9 years ago

Simple Injector will handle this for you transparantly. Simple Injector does not have the notion of root and child containers and scoping is always handled implicitly. Compared to Autofac, it will never allow you to resolve a scoped instance without the existence of an active scope (it will throw an exception, making your code fail fast), while Autofac will resolve a singleton in that case (and Ninject will resolve a Transient), which is hardly ever the right behavior. That's a design flaw in my opinion.

So long story short: Yes, Simple Injector will always resolve the instance from the current request and will throw an exception if there is no request in the current scope.