chromelyapps / Chromely

Build Cross Platform HTML Desktop Apps on .NET using native GUI, HTML5, JavaScript, CSS, Owin, AspNetCore (MVC, RazorPages, Blazor)
MIT License
2.98k stars 279 forks source link

Feature Request: Support custom IServiceProvider factory in DI implementation #271

Closed wwarby closed 3 years ago

wwarby commented 3 years ago

Hi all - just getting started with Chromely, building a UI for a pre-existing project in which I'm already fairly heavily committed to using Autofac as my DI container rather, and had a devil of a time getting it to play nicely with Chromely. It's quite possible there's already an easier solution available than the one I came up with but if not, I'd like to propose a small enhancement that would make this kind of DI container substitution easier. Here's what I think would have made the integration easier:

The problem: Autofac (and possibly other third party IoC libs) provides an implementation of IServiceProvider (AutofacServiceProvider) but no implementation of IServiceCollection so there is no way to create an IServiceCollection from an Autofac container or container builder for use in Chromely's AppBuilder.UseServices method. I can instantiate my own instance of ServiceCollection and add it's services into my Autofac container builder, but there is then no way (that I could find) to use the resulting IServiceProvider with Chromely, because the AppBuilder.Run is coded to construct it's service provider directly from the IServiceCollection. What I needed was a way to define my own function to return a custom implementation of IServiceProvider, effectively replacing the above line. This is what I came up with:

Create a clone of AppBuilder.cs in my own app and make the following changes: Add this:

private Func<IServiceProvider> _serviceProviderFactory;

public AppBuilder UseServiceProviderFactory(Func<IServiceProvider> serviceProviderFactory) {
    _serviceProviderFactory = serviceProviderFactory;
   return this;
}

and replace

 _serviceProvider = _serviceCollection.BuildServiceProvider();

with

_serviceProvider = _serviceProviderFactory != null
    ? _serviceProviderFactory()
    : _serviceCollection.BuildServiceProvider();

Usage is then like this:

// Create a new service collection instance
var services = new ServiceCollection();

MyCustomAppBuilder
    .Create()
    // Use our services collection so that all Chromely registrations are added to it
    .UseServices(services)
        // Use our custom Autofac service provider factory
        .UseServiceProviderFactory(() => {
        // Create an Autofac container
        var builder = new Autofac.ContainerBuilder();

        // Register services with all the power of Autofac
        builder.RegisterModule<MyAutofacModule>();
        builder.Register<MyService>().As<IMyService>();

        // Add Chromely services into the pot
        builder.Populate(services);

        // Build the container
        var container = builder.Build();

        // Return a new Autofac-based provider that can provide both services added
                // to IServiceCollection and services registered directly with Autofac
        return new AutofacServiceProvider(container);
    })
    .UseConfig<DefaultConfiguration>(config)
    .UseApp<MyChromelyApp>()
    .Build()
    .Run(args);

My implementation is obviously quite smelly because I've copied a class from Chromely and taken on responsibility for keeping it up to date, but if the project could provide the UseServiceProviderFactory(Func<IServiceProvider> serviceProviderFactory) I've proposed above, I could get rid of my local class. If the maintainers think this is a viable solution I'd be happy to submit a pull request with the implementation.

mattkol commented 3 years ago

@wwarby Use of external IOC like Autofac is what I worry about too myself at the beginning, but my little research at the time, I felt what we have will take care of that. Apparently not.

I will suggest that we make it a bit more generic and consistent with what we have - pattern wise.

Something like ...

public abstract class ChromelyServiceProviderFactory
{
      public abstract IServiceProvider BuildServiceProvider(IServiceCollection services);
}
public AppBuilder UseServiceProviderFactory(ChromelyServiceProviderFactory serviceProviderFactory)
{
     _serviceProviderFactory = serviceProviderFactory;
      return this;
 }

I just did this, on the fly, I may have missed something...

wwarby commented 3 years ago

Sure that'd work perfectly well and has the added benefit of not requiring the user to instantiate their own ServiceCollection instance (which I don't need to do except for the fact that I needed to get a reference to it when creating my provider). What you've proposed above will definitely facilitate using Autofac as a third party IoC container - I can't speak authoritatively for other containers, but in principal it should work for any container because it should be possible to wrap any arbitrary container in a custom implementation of IServiceProvider even if the container doesn't provide a ready-to-use implementation of the interface. For example:

public class CustomServiceProvider : IServiceProvider {

    private readonly IThirdPartyContainer _container;

    private readonly IServiceProvider _wrappedServiceProvider;

    public CustomServiceProvider(IThirdPartyContainer container, IServiceCollection services) {
        _container = container;
        _wrappedServiceProvider = services.BuildServiceProvider();
    }

    public object GetService(Type serviceType) {
        try {
            return _container.Resolve(serviceType);
        } catch() {
            try {
                return  _wrappedServiceProvider.GetService(serviceType);
            } catch() {
                return null;
            }
        }
    }

}

public class CustomServiceProviderFactory : ChromelyServiceProviderFactory {

    public IServiceProvider BuildServiceProvider(IServiceCollection services) {
        var builder = new ThirdPartyContainerBuilder();
        builder.Register(); // Custom registrations using third party's service registration APIs
        var container = builder.Build();
        return new CustomServiceProvider(container, services);
    }

}

The above is written on the fly so may not be 100% right but I'm pretty sure the principle would hold for any arbitrary IoC container you might want to use.

mattkol commented 3 years ago

Great! Looks like we are on the same page. You suggested submitting a PR, sure you can go ahead with it. Alternatively I can make the change and you help test. Either way we are good. Thanks.

wwarby commented 3 years ago

Great - thanks. I'll put together the change as a PR and test it in my own app to make sure it behaves as expected. Leave it with me ;)

wwarby commented 3 years ago

I've submitted the pull request with the changes exactly as you proposed - I've tested locally and can confirm it works as expected with Autofac, so I'm confident this will give others what they need to be able to integrate others the third party IoC containers. Thanks for collaborating with me on this, I really appreciate it :)

mattkol commented 3 years ago

@wwarby thanks for making Chromely better.