API Proposal: Register injectable services using an attribute

ArthurHNL commented 4 years ago

Background and Motivation

Lot's of applications rely on dependency injection (DI). However, with the current state of dependency injection in .NET Core, DI has to be configured entirely manually with no form of service discovery available. This decreases modularity of the software as this usually (especially in ASP.NET Core applications) results in a single file where all services are added to the service collection.

Therefore, I propose an attribute that can be used to automatically discover injectable services from an assembly, by decorating injectable services with this attribute, increasing modularity. This attribute is inspired by how Nest.js and Angular use a similar pattern.

Especially in team environments this can be helpful, because a single file where everyone is changing stuff can easily become a hotspot for version conflicts.

Proposed API

namespace Microsoft.Extensions.DependencyInjection.Abstractions
{
+    [AttributeUsage(AttributeUsage.Class, AllowMultiple = true)]
+    public sealed class InjectableAttribute : Attribute
+   {
+       public InjectableAttribute(Type? abstractionType = null, ServiceLIfetime lifeTime = ServiceLifetime.Scoped)
+       {
+            this.AbstractionType = abstractionType;
+            this.Lifetime = lifetime;
+       }
+      
+       public Type? AbstractionType { get; }
+       public ServiceLifetime Lifetime { get; }
+   }
}

Usage Examples

public interface ISomeService
{
    Task DoSomethingAsync();
}

[Injectable(abstractionType: typeof(ISomeService))]
public class SomeService : ISomeService
{
    public async Task DoSomethingAync() => await Task.Yield();
}

[Injectable(lifeTime = ServiceLifetime.Singleton)]
public class SomeSingletonObject
{
    public void DoSomething() { }
}

[Controller]
public class SomeApiController : ControllerBase
{
    private readonly ISomeService svc;
    private readonly SomeSingletonObject obj;

    public SomeApiController(ISomeService svc, SomeSingletonObject obj)
    {
        this.svc = svc;
        this.obj = obj;
    }

    [HttpGet]
    public async Task Index()
    {
        var task = this.svc.DoSomethingAsync();
        this.obj.DoSomething();
        await task;
    }
}

Risks

Dependency injection implementations must start supporting this new API.
- When an application consists of multiple assemblies, all assemblies must be loaded in some way before starting to search for classes that are decorated with the new InjectableAttribute.
There might be some security considerations, evil assemblies could "sneak" their way into the dependency injection container.

Note that I do not propose to make any breaking changes to the current way dependency injection is configured; this can be seen as an extension and they could work perfectly side by side: complex services could be configured manually with simple types being added by decorating them with the proposed attribute.

ArthurHNL commented 4 years ago

The label should probably be changed to 'area-Extensions-DependencyInjection'.

FiniteReality commented 4 years ago

I feel that if this should be done, it should be opt-in at the container level. Something like serviceCollection.AddServicesWithAttribute<InjectAttribute>(). For other cases, I feel people should try to use MEF 2, as it's designed (somewhat) explicitly to handle this sort of case.

ChrML commented 4 years ago

Could be useful, however adding this at container level is possible without changing dependency injection at all. You can add an extension method to IServiceCollection called something like:

IServiceCollection AddAttributeDecoratedServices(this IServiceCollection services, Assembly fromAssembly)

The implementation of that method simply enumerates all public classes with that attribute in the provided assembly, and adds them to the container. Should not be much more than 20-40 LOC to implement, and does not need to change any existing code.

I don't see any reasonable way to add this outside of container level, and the feature should be opt-in.

There's a few more considerations/buts for adding this:

Much of the same problem can be solved by adding an extension method in the "module" of your app/library: AddMyServices. This removes the complexity of the library users to know how to add and configure your services (Scoped/Singleton etc...). It's declarative and clean added to Startup.cs, and solves the extra team efforts.
Configuration cannot be solved this way, and requires the old pattern: services.AddMyService(opt => ...); Hence this feature would only be useful for services without any options.
The user may not want to inject all the available services. It gets more unclear what services you actually have, Startup.cs become less declarative by not adding your stuff at feature-level.

davidfowl commented 4 years ago

I agree with the points that @ChrML made. There shall be no built in scanning assemblies by default anywhere in this library. That's one of our principles.

The second point is that declaring attributes on a type doesn't mean it should be registered into a specific service collection instance. Also the order in which dependencies are registered is significant as the DI container preserves order for IEnumerable<T> and takes the last implementation of any service type.

dotnet / runtime