autofac / Autofac.Extensions.DependencyInjection

Autofac implementation of the interfaces in Microsoft.Extensions.DependencyInjection.Abstractions, the .NET Core dependency injection abstraction.
MIT License
195 stars 47 forks source link

Make AutofacServiceProvider and AutofacServiceProviderFactory extensible #20

Closed jeff-winn closed 7 years ago

jeff-winn commented 7 years ago

As an alternative solution to Issue #18 I planned on exposing something we could use internally within the application to keep the use of Autofac only known about in a few key locations within the application when needing to resolve objects using service location.

It would be extremely helpful if the AutofacServiceProvider exposed a protected get only property for the private IComponentContext so the provider would be extensible without requiring a duplicate reference to the same context object in the subclass.

Along with that, the AutofacServiceProviderFactory right now does not offer any means of overriding the type of service provider being returned without having to either 1) duplicate all of the factory code logic, or 2) use encapsulation to wrap the original factory class. Allowing the CreateServiceProvider method to be virtual, or exposing a protected virtual method called by CreateServiceProvider would be most appreciated.

Thanks for your consideration!

tillig commented 7 years ago

I'm trying to figure out what use case you're trying to solve. For the ResolveNamed thing, you could just as easily do:

public static class MyExtensions
{
  public static T ResolveNamed<T>(this IServiceProvider sp, string name)
  {
    return sp.GetRequiredService<ILifetimeScope>().ResolveNamed<T>(name);
  }
}

No need to have anything additional exposed.

However, I still think you may be missing an important design consideration - by extending the abstractions provided by Microsoft.Extensions.DependencyInjection to have implementation-specific behavior (ie, it'd only work with Autofac backing it), you're breaking the whole abstraction. In the places where you'd be calling the custom ResolveNamed extension (or whatever), because it'd only work with Autofac backing it, you may as well directly use an ILifetimeScope or similar thing and just tie to Autofac.

It's kind of like... say you have some interface like IDataContext that is used to abstract away your database connections. The point of the interface is to support the Liskov substitution principle - that you can treat all IDataContext implementations equally, whether they're implemented by a SQL Server, MySQL, or a CSV file. In a case like this, you wouldn't write something like this in your code:

var context = (SqlServerDataContext)this.DataContext;
context.DoSomethingSqlServerSpecific();

You wouldn't write code against the abstraction that requires a specific implementation. That's a design problem.

Anyway, that's why I'm having a difficult time understanding the real goal here. We don't want to break that abstraction. We don't want to make it easy for folks to break the abstraction. We implement the interfaces here - IServiceProviderFactory<T>, IServiceProvider, IServiceScopeFactory - and if you're using this package, it assumes that's what you want - you want support for these interfaces. If you want Autofac specific functions then you'd just use the Autofac interfaces.

Am I missing something? What are you trying to accomplish that can't be accomplished using a mechanism like the above sample extension?

jeff-winn commented 7 years ago

What I was thinking about doing, and what you had done were pretty much the exact same result, just a different way of doing it I suppose.

Microsoft breaks the Liskov substitution rule slightly within their abstraction framework for GetRequiredService since they did have to cast the regular IServiceProvider to the ISupportRequiredService interface but with the extension being designed to work with that particular interface it's neither good nor bad. I was just planning on following suite with what their design pattern was by adding a couple extra interfaces to work with an IServiceProvider to support the extra features that were specific to Autofac.

public interface ISupportNamedService
{
    T GetNamedService<T>(string name);
}

public static class ExtensionMethods
{
    public static T GetNamedService<T>(this IServiceProvider sp, string name)
    {
        return ((ISupportNamedService)sp).GetNamedService(name);
    }
}

I reduced the amount of code to get the point across since it's obviously not production ready like this, but hopefully you'll see where my thoughts were at. The ISupportNamedService interface just needed to be implemented on the AutofacServiceProvider class or a subclass, ergo why I was hoping to see the extensibility changed slightly.

In the end what you proposed and what I was thinking about would do the same thing, with one having more code than the other to maintain over the lifetime of the products. I'll just follow the approach you outlined above since in the end I have no real feelings either way how it's accomplished as I can resolve stuff by name.

Thanks!