Give DI ServiceDescriptor an interface

[thepinkmile]

I am trying to create a wrapper on top of Microsoft.Extensions.DependencyInjection to provide some custom functionality.

What I really need is for the ServiceDescriptor to have an interface of IServiceDescriptor. This is because my custom functionality requires me to store extra information about a service (ServiceName & ScopeName). Currently, to implement my custom descriptors I have to derive from the ServiceDescriptor class which is not ideal as I HAVE to use the relevant constructors for the instance type (discovery of this is currently internal). However, my use case is simple. My CustomServiceDescriptor IS a normal ServiceDescriptor but just with a few extra parameters that I can use both at registration time and for retrieval.

Something like this is what I want to be able to do

public class CustomServiceDescriptor : IServiceDescriptor
{
    public CustomServiceDescriptor(IServiceDescriptor descriptor, MyCustomData metadata)
    {
        _innerDescriptor = descriptor;
        _metadata = metadata;
    }

    // ... All IServiceDescriptor properties just forward to _innerDescriptor

    public MyCustomData Metadata { get => _metadata; }

    private readonly IServiceDescriptor _innerDescriptor;
    private readonly MyCustomData _metadata;
}

Such that if I use my custom descriptors on a common IServiceCollection it will just do what it already does, but if I add this to my custom IServiceCollection it will use the extra metadata when building my custom IServiceProvider.

I can already implement my own IServiceCollection & IServiceProvider to do what I need, but the ServiceDescriptor is causing issues so I have had to make my custom collection and provider accept my own custom Descriptor types which to me feels like a code smell.

My use-case is that I need to register dependencies but only for specific named plugins. So my plugin assembly can only retrieve it's own registered dependencies and those of the "global" scope. This is different than how MVC creates a scope for a request and as such the scoped dependencies are created and disposed of only for that requests pipeline. My plugins are to enable custom functionality within my application and as such each plugin should not have access to other plugin dependencies.

Also it would be nice if this change could be made to both v1.1 & v2.x as some of my projects are still using v1.1 and net452.

I hope I have described this well enough. TIA for any help or advice on this issue.

[pakrym]

It's a massive change to rewire everything to use IServiceDescriptor and the benefit is not obvious.

To help your case static Create method could be added that takes IServiceDescriptor descriptor, MyCustomData metadata and selects and calls appropriate CustomServiceDescriptor ctor.

[thepinkmile]

I have just made a fork of this repo and looking into the changes required. However, I am unsure what you mean by adding a Create method. Could you give me an example implementation that would allow for me to have easy access to the data I need?

If my understanding is correct, I don't think this would work for my case. This is because I might need to add multiple sets of metadata that are unrelated. In my initial post I mention ServiceNameand ScopeName. My CustomServiceDescriptor was just a single impl of one of these (lets just say that it was the ServiceName impl). If I then had another MySecondCustomServiceDescriptor to represent another set of potential metadata (let assume this one is ScopeName). How could I then mix the two? I believe I would have to define another ServiceDescriptor that mixes the two sets of metadata.

Taking my example above I can define any number of metadata sets. Then I can create any mixture of ServiceDescriptor I need (which would be defined by the external plugin and not my internal code). For example:

public static IServiceCollection AddMySpecialType(this IServiceCollection builder, string name, object scope)
{
    var baseDescriptor = ServiceDescriptor.Create<ISpecialInterface,MySpecialType>(ServiceLifetime.Singleton);
    var namedWrapper = NamedServiceDescriptor.Create(baseDescriptor, name);
    var scopedWrapper = ScopedServiceDescriptor.Create(namedWrapper, scope);
    builder.Add(scopedWrapper);
    return builder;
}

or if I wanted to I could do:

public static IServiceCollection AddMySpecialType(this IServiceCollection builder, string name, object scope)
{
    var baseDescriptor = ServiceDescriptor.Create<ISpecialInterface,MySpecialType>(ServiceLifetime.Singleton);
    builder.Add(NamedServiceDescriptor.Create(baseDescriptor, name));
    builder.Add(ScopedServiceDescriptor.Create(baseDescriptor, scope));
    return builder;
}

These are essentially just wrapping types and as such I don't think I should be forced to have to implement the 3 constructors of the ServiceDescriptor in my wrapping types. In the case of the first example above where the descriptor is wrapped twice, if I used the Create methods as mentioned then I would loose the ServiceName information at the point I call ScopedServiceDescriptor.Create.

[izzycoding]

Why has this been closed? I believe this would still be a useful addition and I am currently still working on an implementation for it.

[izzycoding]

Just to note, another issue with deriving from the current ServiceDescriptor means I also inherit the static Create methods. For my current custom descriptors most of these are irrelevant and force me to hide them by re-implementing them.

[muratg]

@izzycoding It's not clear per the thread if this is something we'd like to get in.

cc @davidfowl and @pakrym for their thoughts.

[pakrym]

This change is too large to do even if there were good reasons for it. In my opinion, there are none.

[izzycoding]

Maybe not for the current 3.0. But looking forwards this would allow easy extension of the base framework to allow other potential features. In my case the support for named services (which most non-MS frameworks support). I currently do this by having a custom implementation of an IServiceDescriptor which just adds a name property. I can then implement an ISupportNamedServices interface for IServiceProvider. If that exists and the ServiceDescriptor “is a” INamedServiceDescriptor then internally I can register it as such. I currently have an implementation that extends StructureMap and a few extensions for MS-DI which would allow for this to work as expected. Granted my current impel for MS has some nasty internal dynamic type creation to make the service lookup work properly, but at least it is a start. I hope to have something to show here very soon which may help demonstrate this feature/issue.

[izzycoding]

Too large a change, I have most of my impl done and it doesn’t require much change at all. Mostly just parameters changing to use an interface rather than a static type.

[pakrym]

Too large a change, I have most of my impl done and it doesn’t require much change at all. Mostly just parameters changing to use an interface rather than a static type.

What about supporting new descriptor type in all container that support ASP.NET Core?

In my case the support for named services (which most non-MS frameworks support). I currently do this by having a custom implementation of an IServiceDescriptor which just adds a name property. I can then implement an ISupportNamedServices interface for IServiceProvider. If that exists and the ServiceDescriptor “is a” INamedServiceDescriptor then internally I can register it as such.

I don't see how having ServiceDescriptor be a class with constructors prevents implementing NamedServiceDescriptor

[izzycoding]

NamwdServiceDescriptor was just one example of an extra form. I currently have 5 different extensions which cause issues because they don’t require all the current constructor parameters and having them be required in order to have my custom implementations is a pain as I cannot pass null due to internal checks that throw an ArgumentNullException causing my custom type to fail construction.

Supporting the new descriptor types would be opt-in rather than compulsory. Any existing implementations would initially just use the descriptor as it currently is until they decide to implement them (hence my use of ISupportNamedService which activated the relevant resolution extensions; similar to how ISupportRequiredService works).

[pakrym]

Why do you have to use ServiceDescriptor type in considering that your implementations would not be usable with other service provider implementations (because of unexpected null values)? Just add your own descriptor type and allow it to be created from existing ServiceDescritor.

[izzycoding]

My custom descriptors work perfectly well with existing providers. The fact I don’t require to enter all the parameters is because my custom descriptors effectively replace them internally (in some cases with a Lazy factory methods or dynamic/mocked types). Also, I have a ForwardingDescriptor (something I need because my implementation has the concept of dedicated plugin sudo-scopes which are required to keep each plugins dependencies isolated). This takes 2 service descriptors and depending on their configuration returns a new descriptor that defines the forwarding registration potentially for both the plugin scope and the global scope. This cannot be done with the current constructors. Instead I am having to fake a call that puts an empty factory in the base constructor (or set typeof(object) for the service and implementation types, and then replace it all later in my own constructor. That to me is a code smell and doesn’t sit right. Just by changing to make ServiceDescriptors implement an interface I then get rid of all the rubbish, I am no longer forced to use stupid constructors that don’t do what I need. And I can have a clean API for my extensions without having methods defined that throw NotSupportedExceptions. These changes also simplify my custom feature usage and reduce the code that needs testing and all together reduces the technical debt of my codebase.