Skip to content
This repository was archived by the owner on Nov 2, 2018. It is now read-only.

Make ServiceProvider class public #476

Closed
fredrikhr opened this issue Jan 26, 2017 · 14 comments
Closed

Make ServiceProvider class public #476

fredrikhr opened this issue Jan 26, 2017 · 14 comments
Assignees

Comments

@fredrikhr
Copy link
Contributor

I am writing an extension to the DI system to support Sub-ServiceProviders that are accessible through a Sub-ServiceManager by a specified key (or name).

However, to be able to do that I need to implement a SubServiceProvider class which ideally should work exactly like the normal ServiceProvider, just with the default fallback to ask the root level provider to realize the server if the Sub-ServiceProvider cannot do it.

However, since the ServiceProvider class is internal I cannot derive from it and thus not easily share and extend the feature of the standard DI Provider.

Having the ability for such a nested DI-container could be very beneficial. And although I realize that there are other DI frameworks that support this, I'd really love to build this for the MS DI-system.

If there are big issues with making the ServiceProvider a public class, I am willing to submit a complete PR with support for a Sub-ServiceProvider (and a respective Builder and Manager class) here in this repo.

@davidfowl
Copy link
Member

We don't have plans to add child container support. There's another long thread on that.

However, since the ServiceProvider class is internal I cannot derive from it and thus not easily share and extend the feature of the standard DI Provider.

Can you show what exactly you plan to do if the ServiceProvider was public? I don't quite understand it from the description.

@fredrikhr
Copy link
Contributor Author

Example for a use case of a SubServiceProvider:

public class Startup {
    public void ConfigureServices(IServiceCollection services) {
        services.AddSubServiceManager((ISubServicesManagerBuilder<string> subServiceManager) => {
            subServiceManager.AddSubServices("subServicesName", (IServiceCollection subServices) => {
                subServices.AddSingleton(new MyMagicService());
            });
        });
    }
    public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory) {
        var serviceProvider = app.ApplicationServices;
        var subServiceManager = serviceProvider.GetRequiredService<ISubServiceManager<string>>();
        IServiceProvider subServiceProvider = subServiceManager.GetServiceProvider("subServicesName");
        var myMagicServiceInstance = subServiceProvider.GetRequiredService<MyMagicService>();
    }
}

This way, you could make branches in the DI-container structure, and separate each branch from each other... This makes the following possible:

  • Having multiple Database Providers in a DI container
  • Having DbContexts with different sources (as in migration applications that move data from one DB to another)
  • This would also allow for hosting Web Applications with small modules within the same application scope, where each module could be its own independant application

But regardless of what I personally think could be done by extending the default service provider, I think it is generally a good idea to allow people to extend this class, especially since DI has become essential to ASP.NET Core applications.

@fredrikhr
Copy link
Contributor Author

@davidfowl And regarding the child container support: I am implementing that feature for my own use right now anyways, but it depends on the Default Service Provider being a public class so I can derive from it. And if you like am very happy to make a PR implementing Child Container support.

But I am not planning on writing my own DI container system just to have child containers. Not when I get a perfectly sound and easy to use DI system here already.

@davidfowl
Copy link
Member

@couven92 Can you show the code that overrides the DefaultServiceProvider

@fredrikhr
Copy link
Contributor Author

@Eilon
Copy link
Member

Eilon commented Jan 31, 2017

@davidfowl when we talked you said there might be a simpler way to achieve this - can you share?

@davidfowl
Copy link
Member

@couven92 Since you're using functionality that is public, why not just make another class that derived from IServiceProvider and use BuildServiceProvider to make the "child container"?

public class SubServicesServiceProvider : IServiceProvider
{
    private readonly IServiceProvider rootServiceProvider;
    private readonly IServiceProvider subServiceProvider;

    public SubServicesServiceProvider(IServiceProvider rootServiceProvider, IServiceCollection subServiceCollection, bool validateScopes)
    {
        this.rootServiceProvider = rootServiceProvider;
        this.subServiceProvider = subServiceCollection.BuildServiceProvider(validateScopes);
    }

    public object GetService(Type serviceType) => subServiceProvider.GetService(serviceType) ?? rootServiceProvider?.GetService(serviceType);
}

@fredrikhr
Copy link
Contributor Author

Initially, I thought so as well, but then it is not possible to make the child container inherit the root container.

With the model I am proposing, it will be possible to have a child service that requires services from the root provider.

@davidfowl in the example you show, child services will be realized only by the created child service provider. In my example service realization will propagate upwards through to the root provider.

The override of the GetServices method enables us to make an actual connection between the child and the root provider

@davidfowl
Copy link
Member

With the model I am proposing, it will be possible to have a child service that requires services from the root provider.

I don't really understand how that would work with your code. Can you show a concrete example? I don't see what functionality you're using in the derived container to enable that. The container itself doesn't call GetService so there's no chaining that will happen.

@fredrikhr
Copy link
Contributor Author

Consider the following:
I have one class, the RootService which will be added to the root service provider:

public class RootService {}

Now we have another class, SubService, where the ctor gets a RootService instance as argument.

public class SubService {
    private readonly RootService rootServiceInstance;

    public SubService(RootService rootServiceInstance) {
        this.rootServiceInstance = rootServiceInstance;
    }
}

Now those are added to DI as services (for example in a WebApp):

public class Startup {
    public void ConfigureServices(IServiceCollection services) {
        services.AddSingleton<RootService>();
        services.AddSubServicesManager(subServiceManager => {
            subServiceManager.AddSubServices("subServicesName1", subServices => subServices.AddSingleton<SubService>());
            subServiceManager.AddSubServices("subServicesName2", subServices => subServices.AddSingleton<SubService>());
         });
     }
     public void Configure(IApplicationBuilder app, IHostingEnvironment env, ILoggerFactory loggerFactory) {
        var serviceProvider = app.ApplicationServices;

        var subServiceManager = serviceProvider.GetRequiredService<SubServicesManager>();
        IServiceProvider subServiceProvider1 = subServiceManager.GetServiceProvider("subServicesName1");
        IServiceProvider subServiceProvider2 = subServiceManager.GetServiceProvider("subServicesName2");

        var subServiceInstance1 = subServiceProvider1.GetRequiredService<SubService>();
        var subServiceInstance2 = subServiceProvider2.GetRequiredService<SubService>();
    }
}

Now @davidfowl, in the code you suggested, the RootService instance can only be realized from the rootServiceProvider. This will lead to an exception when your subServiceProvider attempts to realize the SubService instance.

With the override of GetServices as I suggest, a non realizable service in the child container will automatically propagate to the root container. That is because the ServiceProvider internally will call the GetService method to fill in the argument for the SubService constructor

@davidfowl
Copy link
Member

That is because the ServiceProvider internally will call the GetService method to fill in the argument for the SubService constructor

Are you sure about this? The service provider doesn't call GetService internally at all AFAIK.

Here's my sample that shows it doesn't work, you can tell me what pieces of the code are broken:

0444a5d

Let me know what I missed.

@davidfowl
Copy link
Member

Ping @couven92

@fredrikhr
Copy link
Contributor Author

Sorry, I had to deal with a lot of University stuff this week...

I looked at the error of the test @davidfowl referred to and it turns out that I was wrong, the ServiceProvider class does not call GetService when realizing the constructor parameters.

Sadly this means that a nested Child ServiceProvider will be a little bit more difficult do implement. Since the nested service provider was my motivation to make the default service provider public, we could now close this issue (and perhaps open it again later). I still believe that making the class public could be interesting in other scenarios as well, but I have no concrete examples where that would be useful. In case we ever decide to make it public, PR #477 is ready to be merged with these changes.

However, I am still interested in implementing a nested Child Service Container and integrating it into DI, so I will open a new issue and prepare a PR for that. I will reference this issue there.

@davidfowl
Copy link
Member

davidfowl commented Feb 11, 2017

We're not going to make it public as there isn't any interesting scenarios as of right now. Also we're not adding support for child containers (we've discussed it at length on another issue see #246) so I'd suggest forking or using another container like autofac which already has this feature.

Closing his isssue out.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants