[API Proposal]: Hooking into service construction in default DI implementation

Danielku15 commented 2 months ago

Background and motivation

Architectures and principles can differ from project to project and when it comes to centralizing some aspects of your services the DI container is often a first point to handle these aspects.

While I can understand that the extensions team doesn't want to add a central abstraction and implementation for systems like property injection ^1 ^3, I cannot fully support this decision. Microsoft libraries often rather try to be not "opinionated" but it seems in this case we are lacking the right extension point for developers to add support for such a mechanism without bloating the core. It seems like an extreme measure to fully swap out the DI container because some additional customization aspects are needed.

So I was thinking of an alternative approach to tackle this topic: What if the default DI container would provide an extension point where developers or library authors can hook into the service creation? Such a callback would open the door for a nice range of extensions:

Injecting dependencies via properties.
Calling initialization methods after the constructor if certain interfaces are implemented (similar to Angulars OnInit)
Providing interceptors/decorators to services ^6.

This is a similar strategy like EF Core^5 follows. With interceptors the main library can be augmented and extended with new aspects without fully exchanging central bits (like the requiring to exchange the provider).

API Proposal

I am not fully sure about the API design itself, I'm open for any alternative approach providing the same result.

As main API proposal I would see: Add an option for a callback which the DI container calls when a service was realized.

namespace Microsoft.Extensions.DependencyInjection;

public delegate object? ServiceRealizedCallback(IServiceProvider serviceProvider, ServiceDescriptor 
 serviceDescriptor, object? realizedService);

public class ServiceProviderOptions
{
    public ServiceRealizedCallback? OnServiceRealized { get; set; }
}

API Usage

interface IServiceInit { void OnInit(); }
class MyServiceWithPropertyDependencies : IServiceInit 
{
    public required IOtherService OtherService {get;set;}
    public IOptionalService? OptionalService {get;set;}
    void OnInit() { OptionalService ??= OtherService?.OptionalService; }
}

var services = new ServiceCollection();
services.AddSingleton<MyServiceWithPropertyDependencies>();
services.AddSingleton<IOtherService, OtherService>();

var serviceProvider = services.BuildServiceProvider(new ServiceProviderOptions
{
    OnServiceRealized = (sp, desc, obj)
    {
        if (obj != null)
        {
            obj = ApplyPropertyDependencyInjection(sp, desc, obj);
        }
        if (obj is IServiceInit init)
        {
            init.OnInit(); 
        }
        return obj;
    };
});
var instance = serviceProviderFactory.GetRequiredService<MyServiceWithPropertyDependencies>();
// instance.OtherService != null
// instance.OptionalService == null

Alternative Designs

Ad 1: The ServiceProviderEngine and default implementation could be made public or be exposed with another abstraction. I don't like this approach so much as it pulls quite many internals but its still a legit one. It might make it more complicated for others to extend the default behavior as the whole engine needs to be substituted.

Ad 2: Instead of a simple delegate we could also define an interface and allow multiple "interceptors" to be registered. This would allow an easier extension through extension methods and external libraries:

namespace Microsoft.Extensions.DependencyInjection;

public interface IServiceRealizationInterceptor
{
   object? OnServiceRealized(IServiceProvider serviceProvider, ServiceDescriptor 
 serviceDescriptor, object? realizedService);
}

public class ServiceProviderOptions
{
    public IList<IServiceRealizationInterceptor> Interceptors { get; } = new List<IServiceRealizationInterceptor>();
}

While from an API design it might be better for extension, it has more performance and complexity implications (order of interceptors, performance with looping and interface calls, chicken-egg problem if people want to use DI already in the interceptors).

Risks

The DI container is a perfromance sensitive aspect in most applications. Poorly written interceptors could result in a performance degredation which is percieved as bad performance of the DI framework itself.

Unexperienced users might not understand that misbehaviors might be caused by interceptors resulting in additional issues being raised against the core library while 3rd party extensions are to blame. With the .net hostbuilders its easy to hide things behind the scenes.

dotnet-policy-service[bot] commented 2 months ago

Tagging subscribers to this area: @dotnet/area-extensions-dependencyinjection See info in area-owners.md if you want to be subscribed.

julealgon commented 2 months ago

Very strongly against this proposal as it just pushes people into the pit of failure. Property injection is an abomination that should never be used unless one is dealing with a super legacy framework that does not support proper constructor injection (even WebForms supports ctor-based injection now, so I don't even have an example anymore).

The example's IServiceInit interface is especially egregious as it goes against the correctness-by-construction principle and introduces temporal/sequential coupling to whoever uses it.

I hope this does not move forward.

Danielku15 commented 2 months ago

@julealgon I'm hiding this reply behind an expander as it is rather a direct counter-statement to your opinion than adding much value to the general proposal discussion. I'd like to emphasize some values of the .net foundation code of conduct:

Being respectful of differing opinions, viewpoints, and experiences
Giving and gracefully accepting constructive feedback

Different opinions count, same as yours. I think an API proposal like this is not the right forum to discuss architectural design opinions. There is a good reason this proposal aims to open for extension and not extend the main library. If somebody decides to allow aspects in their architectures its their choice. I see this from an "open-closed priniciple" standpoint and this proposal aims to allow easier extension without exchanging the whole DI container. The extensions are the legit choice of developers authoring their software.

Expand to see

## On your reply and this proposal It seems you only heard "property injection" and completely ignored the nature of this proposal. Its fine if you don't want to use property injection. I am not proposing that property injection is added to the core of the framework or enabled by default. This proposal enables others, who want to go this path, to inject custom logic as per their needs. A "my way or no way" mentality is the death of any innovation. If you don't like it, don't use it and go alternative solution paths. It's fine. ## On property injection Even though I don't want to push for property injection in the base library, I want to have some words on this topic: IMO the Microsoft dependency injection framework is violating the `required` modified which was introduced in C# 11. A DI framework should construct services (or any objects) following its contract of construction. While this contract might be defined within a project specifically, the required modified is a contract built into the language. Still I'm fine if the default DI container doesn't support and respect this contract. But not giving people a way of adding their needs sounds wrong to me. Looking at the market property injection is widely supported: * .net: [AutoFac](https://autofac.readthedocs.io/en/latest/register/prop-method-injection.html) * .net [Dryloc](https://github.com/dadhi/DryIoc/blob/master/docs/DryIoc.Docs/RegisterResolve.md#dryioc-glossary) * .net [lamar](https://jasperfx.github.io/lamar/guide/ioc/setter-injection.html#setter-injection) * .net [stashbox](https://z4kn4fein.github.io/stashbox/docs/configuration/registration-configuration#property--field-injection) * ASP.net Controllers core [fakes it](https://github.com/dotnet/aspnetcore/blob/6b9bba1472da66a9f02ae3cd0f564ad6e2bb5fff/src/Mvc/Mvc.Core/src/ControllerBase.cs#L143) via Service Locator Anti-Pattern * Angular [inject](https://angular.io/api/core/inject) - Here it is even hyped. IMO with nullable reference types and the required modifier in the language having property injection is perfectly fine. These mechanisms define the contract of constructing an object and no matter if you create an object with `new` or via DI container they should be filled. Whether it _should_ be used and in what cases it should be allowed is an architectural decision. It must not be dictated by a building block or a general purist mindset. ## `IServiceInit` > The example's IServiceInit interface is especially egregious as it goes against the correctness-by-construction principle and introduces temporal/sequential coupling to whoever uses it. You are assuming too much here due to the simplistic nature of the example. With constructor injection + property injection (assuming required) the object is correctly **constructed**. But depending on the nature of your object and your application architecture you still might want to run some code **after** your object is constructed. This logic is again depending on the nature of the object/service. The service can describe a contract on how to use it. Such hooks simply make the users life easier. e.g. there is also a big debate on having the possibility of async code in your constructor and rather making async initializers. IMO it is a bad practice to run too much logic inside the constuctor. But the DI container can still serve some as some central place for initialization aspects. e.g. For resilient systems DI container could centrally retry certain initializations depending on errors or even fallback to an alternative service implementation if the initialization of the standard service is currently not possible. Some people might prefer the OOP approach of encapsulating this logic into a special service implementation delegating to others through composition, and others might prefer the AOP approach of augmenting/decorating the implementation. Neither apporach is "wrong" by-design. * Angular: [OnInit](https://v17.angular.io/api/core/OnInit) * WinForms: [Load](https://learn.microsoft.com/en-us/dotnet/api/system.windows.forms.form.load?view=windowsdesktop-8.0) * Wpf: [Loaded](https://learn.microsoft.com/en-us/dotnet/api/system.windows.frameworkelement.loaded?view=windowsdesktop-8.0&redirectedfrom=MSDN) and [Initialized](https://learn.microsoft.com/en-us/dotnet/api/system.windows.frameworkelement.initialized?view=windowsdesktop-8.0#system-windows-frameworkelement-initialized)

dotnet / runtime