Reconsider property injection with required properties

[AntMaster7]

Feature requests for property injection have been made several times in the past and they were always closed. However, now that we have support for required properties in C# I think it would make perfect sense to support them through the default DI container in dotnet core.

If thats definitely not gonna be supported it would be nice to have the default DI container designed such that it could be extended by inheriting from it to add this functionailty in customer projects. The current implementation of the DI container registers itself as the implementation for the IServiceProvider making it impossible to extended the default service provider.

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

Issue Details

---

Feature requests for property injection have been made several times in the past and they were always closed. However, now that we have support for required properties in C# I think it would make perfect sense to support them through the default DI container in dotnet core.

If thats definitely not gonna be supported it would be nice to have the default DI container designed such that it could be extended by inheriting from it to add this functionailty in customer projects. The current implementation of the DI container registers itself as the implementation for the IServiceProvider making it impossible to extended the default service provider.

Author:	AntMaster7
Assignees:	-
Labels:	untriaged, area-Extensions-DependencyInjection
Milestone:	-

[steveharter]

Can you provide a proposed API?

I assume by adding protected members to DefaultServiceProviderFactory.

This issue has been marked needs-author-action and may be missing some important information.

[AntMaster7]

@steveharter My idea would be to have a protected virtual OnRealize method on the ServiceProvider (removing the sealed modifier) that gets called whenever a service is actually "realized" but not when its loaded from the cache (scope). This would then allow one to create a decorated service provider like so:

public class DecoratedServiceProvider : ServiceProvider
{
    public DecoratedServiceProvider(ICollection<ServiceDescriptor> serviceDescriptors, ServiceProviderOptions options) : base(serviceDescriptors, options)
    {
    }

    protected override void OnRealize(Type serviceType, object service)
    {
        // Decorate with for example property injection
    }
}

A simple implementation of the IServiceProviderFactory can then return this custom service provider instance.

To make this work in a local demo I have changed the constructor of the ServiceProvider such that it uses the actual instance type as implementation for the internal IServiceProvider registration:

[RequiresDynamicCode(RequiresDynamicCodeMessage)]
protected internal ServiceProvider(ICollection<ServiceDescriptor> serviceDescriptors, ServiceProviderOptions options)
{
    // ...
    CallSiteFactory = new CallSiteFactory(serviceDescriptors);
    // The list of built in services that aren't part of the list of service descriptors
    // keep this in sync with CallSiteFactory.IsService
    CallSiteFactory.Add(typeof(IServiceProvider), new ServiceProviderCallSite(GetType())); // CHANGE HERE
    CallSiteFactory.Add(typeof(IServiceScopeFactory), new ConstantCallSite(typeof(IServiceScopeFactory), Root));
    CallSiteFactory.Add(typeof(IServiceProviderIsService), new ConstantCallSite(typeof(IServiceProviderIsService), CallSiteFactory));
    // ...
}

I have the modified the internal CreateServiceAccessor method like this:

protected virtual void OnRealize(Type serviceType, object service) { }

// ....

[RequiresDynamicCode(RequiresDynamicCodeMessage)]
private Func<ServiceProviderEngineScope, object?> CreateServiceAccessor(Type serviceType)
{
    ServiceCallSite? callSite = CallSiteFactory.GetCallSite(serviceType, new CallSiteChain());
    if (callSite != null)
    {
        DependencyInjectionEventSource.Log.CallSiteBuilt(this, serviceType, callSite);
        OnCreate(callSite);

        // Optimize singleton case
        if (callSite.Cache.Location == CallSiteResultCacheLocation.Root)
        {
            object? value = CallSiteRuntimeResolver.Instance.Resolve(callSite, Root);

            if(value is not null)
            {
                OnRealize(serviceType, value);
            }

            return scope => value;
        }

        return (ServiceProviderEngineScope engineScope) =>
        {
            var previousValue = callSite.Value;
            var service = _engine.RealizeService(callSite)(engineScope);

            if (service is not null && service != callSite.Value)
            {
                OnRealize(serviceType, service);
            }

            return service;
        };
    }

    return _ => null;
}      

The ReplaceServiceAccessor method would have to be changed accordingly.

The only use case I see for this proposed change is to allow users to implement property injection on top of the existing infrastructure with minimal effort. This could be documented and would in a way add indirect support for property injection. However, most people will probably not use this "extended api" and this begs the question whether wrapping the RealizeService call into a closure is a performance issue. Although it could probably be made conditional by having an additional constructor thats protected instead of the "main" constructor and setting some flag when the class is inherited and only then wrap the RealizeService call into a closure.

[davidfowl]

I don’t think this should be an extensibility point. We either do this feature or we don’t. I’d like to understand if other container authors are headed down this route before we do anything.

[AntMaster7]

@dotnetjunkie @tillig What do you think about whether dotnet should support property injection or not with required properties? Both of your containers support this and you're experts on the area of DI containers.

[davidfowl]

@z4kn4fein
@dadhi
@ipjohnson
@alexmg
@tillig
@pakrym
@ENikS
@seesharper
@jeremydmiller
@alistairjevans
@halter73
@steveharter

Supporting property injection is already a common use case in Blazor. I don't see a single reason why this shouldn't be supported as a general feature of the default DI.