Skip to content
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

[API Proposal]: ServiceCollection support post service registration #110926

Open
Li7ye opened this issue Dec 24, 2024 · 8 comments
Open

[API Proposal]: ServiceCollection support post service registration #110926

Li7ye opened this issue Dec 24, 2024 · 8 comments
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-DependencyInjection needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration

Comments

@Li7ye
Copy link

Li7ye commented Dec 24, 2024

Background and motivation

I'm a framework programmer. Usually, I often add extension method for IServiceCollection. But sometimes, my extension method is required to know IServiceCollection state.

For instance, services.AddUnitOfWork() need to know all DbContextOptions in services and update DbContextOptions. if services.AddUnitOfWork() is added after registration of all dbcontexts, it can work properly. Otherwise, can get side effect(unexpected behavior). Some users maybe call services.AddUnitOfWork before or between dbcontexts registration by accident, because AddUnitOfWork is built on IServiceCollection and there is no restriction.

// work properly
services.AddDbContext<DB1>();
services.AddDbContext<DB2>();
services.AddUnitOfWork();

// side effect
services.AddDbContext<DB1>();
services.AddUnitOfWork();
services.AddDbContext<DB2>();

// can't work
services.AddUnitOfWork();
services.AddDbContext<DB1>();
services.AddDbContext<DB2>();

API Proposal

I suggest has an interface can be invoked after all services registered but before build to provider.

public interface IPostServiceRegistration
{
      void AddService(IServiceCollection services);
}

API Usage

services.AddPostRegistration<MyPostRegistration>()

Hence services.AddUnitOfWork can use the new feature to register own services or add validation, and can be added out of order by user. User only know I need to call this method without other knowledge.

Alternative Designs

No response

Risks

No response

@Li7ye Li7ye added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Dec 24, 2024
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Dec 24, 2024
@KalleOlaviNiemitalo
Copy link

For instance, services.AddUnitOfWork() need to know all DbContextOptions in services and update DbContextOptions.

Can you update the options by registering a generic service for typeof(IDbContextOptionsConfiguration<>)?

@Li7ye
Copy link
Author

Li7ye commented Dec 25, 2024

This sounds like an abuse of the service locator?

@colejohnson66 Understand. To be honest, this feature is not for most developers. If package author can follow the convention, the issue would go away. For example, service.AddMvc() return IMvcBuilder, if someone want to extend MVC feature, he can add extension method on IMvcBuilder rather than IServiceCollection. But in real world, most return type is IServiceCollection after calling service.AddXXX().

@Li7ye
Copy link
Author

Li7ye commented Dec 25, 2024

For instance, services.AddUnitOfWork() need to know all DbContextOptions in services and update DbContextOptions.

Can you update the options by registering a generic service for typeof(IDbContextOptionsConfiguration<>)?

If my understand is correct, you means Options Pattern?
If yes, DbContextOptions is not generic options. EF Core need to register DbContextOptions in DI.

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Dec 25, 2024

@Li7ye, IDbContextOptionsConfiguration<TContext> was added in dotnet/efcore#32518 and shipped in Entity Framework Core 9.0.0.

In EntityFrameworkServiceCollectionExtensions, all AddDbContext methods eventually call AddCoreServices, which registers DbContextOptions<TContextImplementation> as a service with a factory delegate that calls CreateDbContextOptions<TContextImplementation>, which gets all IDbContextOptionsConfiguration<TContext> services from the IServiceProvider and calls their Configure methods. Typically, the IDbContextOptionsConfiguration<TContext> service is registered by ConfigureDbContext<TContext>, and this registration applies only to the specific TContext type; but I think you could bypass that and register a service for the IDbContextOptionsConfiguration<> open generic type instead, so that it would apply to every TContext.

@KalleOlaviNiemitalo
Copy link

KalleOlaviNiemitalo commented Dec 25, 2024

public interface IPostServiceRegistration
{
      void AddService(IServiceCollection services);
}

It looks like this API proposal is not complete and one would also need to add at least the following:

namespace Microsoft.Extensions.DependencyInjection
{
    public partial class ServiceCollectionServiceExtensions
    {
        public static IServiceCollection AddPostRegistration<T>(
            this IServiceCollection services)
            where T : IPostServiceRegistration, new();
    }
}

And perhaps also:

namespace Microsoft.Extensions.DependencyInjection
{
    public partial class ServiceCollectionServiceExtensions
    {
        public static IServiceCollection AddPostRegistration(
            this IServiceCollection services,
            IPostServiceRegistration registration);
    }
}

And designs for:

  • How would the IPostServiceRegistration implementations be stored in the IServiceCollection? Possibilities:
    • a ServiceDescriptor even though the IPostServiceRegistration is not intended to be queried from IServiceProvider
    • a new interface implemented by the same class as IServiceCollection; if that interface is not implemented, then AddPostRegistration throws
    • ConditionalWeakTable<IServiceCollection, some new type>
  • How does this work when a third-party DI container is used? What calls the IPostServiceRegistration.AddService methods?
    • the third-party DI library calls them; then M.E.DI must define API for locating them
    • M.E.DI calls them before third-party DI takes control; may be difficult
  • Are the IPostServiceRegistration.AddService methods given the original IServiceCollection, or a copy of it? This affects what happens if multiple IServiceProvider instances are built from the same IServiceCollection.

@KalleOlaviNiemitalo
Copy link

The new IPostServiceRegistration interface feels a bit unnecessary; Action<IServiceCollection> could be used instead. This change would result in shorter C# code in the AddPostRegistration call. Interfaces make sense in IConfigureOptions<TOptions> etc. because they allow other services to be injected to the constructor of the class that implements the interface; but IPostServiceRegistration would be used before the service provider is built, so the services are not yet available for injection.

@Li7ye
Copy link
Author

Li7ye commented Dec 26, 2024

this registration applies only to the specific TContext type; but I think you could bypass that and register a service for the IDbContextOptionsConfiguration<> open generic type instead, so that it would apply to every TContext.

Thanks for you feedback, because of some restrictions, I can't upgrade EF Core to 9.0. And, EF Core is one of the examples.
Currently, the workaround is that use code analyzer to relieve this issue and it brings more maintenance cost :(

@steveharter
Copy link
Member

@steveharter steveharter added needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration and removed untriaged New issue has not been triaged by the area owner labels Dec 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-suggestion Early API idea and discussion, it is NOT ready for implementation area-Extensions-DependencyInjection needs-further-triage Issue has been initially triaged, but needs deeper consideration or reconsideration
Projects
None yet
Development

No branches or pull requests

3 participants