Add decorator extension methods to IServiceCollection

[davidfowl]

AB#1244416 Currently Scrutor has these:

https://github.com/khellang/Scrutor/blob/338a59333c7eafe25d5aefdd22434653c83eb9ab/src/Scrutor/ServiceCollectionExtensions.Decoration.cs#L10

I think we should consider adding something similar in the box for a couple of reasons:

• It helps when default implementations are internal or non virtual.
• It promotes composition over inheritance

This needs to be built in a way that doesn't require container authors to change anything (that's why extension methods are being proposed).

cc @khellang Since he may want to contribute this 😉.

Some things I am concerned about in the above implementation:

• Performance of those methods, some of them allocate (https://github.com/khellang/Scrutor/blob/338a59333c7eafe25d5aefdd22434653c83eb9ab/src/Scrutor/ServiceCollectionExtensions.Decoration.cs#L249)
• Performance of Scanning the IServiceCollection, multiple times (https://github.com/khellang/Scrutor/blob/338a59333c7eafe25d5aefdd22434653c83eb9ab/src/Scrutor/ServiceCollectionExtensions.Decoration.cs#L294)
• Performance of service resolution https://github.com/khellang/Scrutor/blob/338a59333c7eafe25d5aefdd22434653c83eb9ab/src/Scrutor/ServiceCollectionExtensions.Decoration.cs#L333-L356, ActivatorUtilities.GetServiceOrCreateInstance should be swapped with CreateFactory.
• The quirks around the fact that this mutates the service collection in place (https://github.com/khellang/Scrutor/blob/338a59333c7eafe25d5aefdd22434653c83eb9ab/src/Scrutor/ServiceCollectionExtensions.Decoration.cs#L296). It affects code like this: Works

  services.AddSingleton<IFoo, Foo>();
  services.Decorate<IFoo, Foo2>();

  Does not work

  services.Decorate<IFoo, Foo2>();
  services.AddSingleton<IFoo, Foo>();

[khellang]

Yeah... The current implementation is a giant hack around the container's current limitations and there are some confusing aspects around the API, like "what lifetime should the decorator have?". I've settled on copying the lifetime of the decoratee, but some people have argued that there are cases where this isn't the best solution.

Agreed on all the perf concerns. I've basically just kept things as simple as possible (code-wise) since it's mostly a one-time cost at startup and no one has complained (yet).

Most of them should be pretty easy to fix, i.e. remove LINQ usage, but some things are just fundamentally hard to do on top of the current feature set. I'd be really interested in hearing your take on the last bullet point, as an example.

[davidfowl]

Yeah... The current implementation is a giant hack around the container's current limitations and there are some confusing aspects around the API, like "what lifetime should the decorator have?". I've settled on copying the lifetime of the decoratee, but some people have argued that there are cases where this isn't the best solution.

I think people are wrong there, the most correct thing is to preserve the lifetime.

Agreed on all the perf concerns. I've basically just kept things as simple as possible (code-wise) since it's mostly a one-time cost at startup and no one has complained (yet).

People wouldn't complain but I would 😄. More seriously, we're been taking a look at how much time we spend re-enumerating the IServiceCollection at startup and it's kinda crazy.

Most of them should be pretty easy to fix, i.e. remove LINQ usage, but some things are just fundamentally hard to do on top of the current feature set.

Agreed but I think doing this without it being a container native feature is appealing.

I'd be really interested in hearing your take on the last bullet point, as an example.

I think that limitation is fine. The alternatives I can think of have to do with storing that list of decorators in a side object and writing a custom IServiceProviderFactory that unwraps them but that seems like overkill.

[davidfowl]

cc @aspnet/di-council

[seesharper]

@aspnet/di-council Better decorator support would be great to have and this is something that has been supported in LightInject since the beginning.

To get the discussion going I'll give you a brief explanation of how this is implemented in LightInject.

Decorator characteristics

• Decorators MUST implement the same interface as the service being decorated.
• Decorators MUST inject the service being decorated.
• Decorators can OPTIONALLY inject other services.
• Decorators ALWAYS follow the same lifetime/lifecycle as the decorated service (decoratee).
• Decorators are ALWAYS applied in their registration order.

Example

public class FooDecorator : IFoo
{
    public FooDecorator(IFoo decoratee, IBar bar)
    {
    }
}

Registration

Let's take a look at the following registrations.

container.RegisterScoped<IFoo, Foo>();
container.Decorate<IFoo, FooDecorator>();
container.Decorate<IFoo, AnotherFooDecorator>();

The decorators are applied in the registration order meaning that the decorator chain here will be

AnotherFooDecorator -> FooDecorator -> Foo

Please note that service registration is completely decoupled from the decorators meaning that the following will also work.

container.Decorate<IFoo, FooDecorator>();
container.Decorate<IFoo, AnotherFooDecorator>();
container.RegisterScoped<IFoo, Foo>();

Lifetime

Decorators follow the same lifetime as the service being decorated which is a very important aspect of decorators. Allowing a decorator to have a different lifetime/lifecycle than the service being decorated could lead to some serious confusion among users. If we think of decorators as being the manifestation of the Open/Closed principle, we want to be able to add new functionality/aspect without altering the behavor of the service being decorated.

If in any case we should require different lifetimes, and mind that these cases should be extremely rare, we can always do "manual" decoration in the factory delegate used during registration. So my advice here is to keep it simple and let decorators follow the lifetime of the decorated service.

Summary

This covers the very basics of decorator support in LightInject and I have intentionally not gone into more advanced scenarios like decorating open generic types which is where things really starts to get complicated from an implementation standpoint. I would also advice against such advanced features as it would make it substantially harder to confirm to the MS.DI abstraction

[davidfowl]

@seesharper See the code I linked to (@khellang's code) which implements decorators on top of the IServiceCollection primitives. I'm entertaining this approach because it doesn't require any changes to the spec or other containers. It's built on top but comes with some limitations.

[dadhi]

@davidfowl ,

If the Decorators implemented externally as extensions, does it mean that DI adapters treat them as normal services and register as usual? As a result we are getting two types of decorators - black boxes in the ServiceCollection and the native ones in IoC libraries.

It may cause a confusion:

• Both Decorartor kinds will have a different feature set.
• Registration intent is erased in case of SC.
• Resolution Error messages may be confusing with the mixed stack of things.

[ENikS]

Could you provide a link to the relevant documentation? This term is rather ambiguous and would be nice to know exactly what is being discussed.

Is this akin to interception?

[khellang]

https://en.m.wikipedia.org/wiki/Decorator_pattern

[ENikS]

So, essentially decorators are what Unity and Castle call interception, or are these separate services?

In other words, if I resolve IFoo, will container return three services with two of them being decorators, or it will return one latest decorator with others and original service chained down the line?

[khellang]

So, essentially decorators are what Unity and Castle call interception, or are these separate services?

I'm not super familiar with it, but I always thought interception was some sort of AOP thing where proxies were generated to intercept calls to resolved instances. I guess you could look at it as interception, but with way less magic.

In other words, if I resolve IFoo, will container return three services with two of them being decorators, or it will return one latest decorator with others and original service chained down the line?

The latter. You'll get a chain of wrapped interceptors.

[ENikS]

Well, if this is the later than it would be rather challenging to implement for multiple, chained decorators. Not impossible, but very challenging. At present Unity does not implement multilevel interception.

I like the idea though

[ENikS]

@dadhi

If the Decorators implemented externally as extensions, does it mean that DI adapters treat them as normal services and register as usual?

I do not think you could register decorators as normal services. The adapter has to recognize these registrations are decorators and create relevant internal registrations (configure interception in case of Unity). The entry in a collection should unambiguously identify it as a decorator.

[khellang]

I do not think you could register decorators as normal services.

The OP shows an implementation of this on top of the existing abstractions as extensions.

[ENikS]

@khellang

I was talking about implementing decoration in the container itself. Done at the container level it would eliminate all these scans and speed things up a bit.

[seesharper]

Is this akin to interception?

You could say that decorators are a way of implementing AOP (Aspect Oriented Programming) where each decorator is responsible for an "aspect" such as logging, profiling ,caching, circuit breaking and other cross cutting concerns. The decorator pattern really starts to shine when dealing with reusable open generic types. As an example, a common way of implementing the command part of the CQRS pattern is to have an ICommandHandler<T> interface like this.

public interface ICommandHandler<in TCommand>
{              
    Task HandleAsync(TCommand command, CancellationToken cancellationToken = default);
}

public class SaveCustomerCommandHandler : ICommandHandler<SaveCustomerCommand>
{
    private readonly IDbConnection dbConnection;

    public SaveCustomerCommandHandler(IDbConnection dbConnection)
    {
        this.dbConnection = dbConnection;    
    }

    public async Task HandleAsync(SaveCustomerCommand command, CancellationToken cancellationToken = default(CancellationToken))
    {
        // Save the customer to the database
    }
}

Now let's imagine that we have many of these command handlers and we want to make sure that we execute them in the context of a transaction. We could start the transaction in each command handler or we could use the decorator pattern to wrap all command handlers in a transaction like this.

public class TransactionalCommandHandler<TCommand> : ICommandHandler<TCommand>
{
    private readonly IDbConnection dbConnection;
    private readonly ICommandHandler<TCommand> commandHandler;    

    public TransactionalCommandHandler(IDbConnection dbConnection, ICommandHandler<TCommand> commandHandler)
    {
        this.dbConnection = dbConnection;
        this.commandHandler = commandHandler;        
    }

    public async Task HandleAsync(TCommand command, CancellationToken cancellationToken)
    {        
        using (var transaction = dbConnection.BeginTransaction())
        {
            await commandHandler.HandleAsync(command, cancellationToken);
            transaction.Commit();     
        }
    }
}

To decorate each command handler with the TransactionalCommandHandler all we need is this line (LightInject).

container.Decorate(typeof(ICommandHandler<>), typeof(TransactionalCommandHandler<>))

And Viola, all command handlers are now executed in a transaction. We could do the same for logging, profiling and other cross cutting concerns. @khellang You can decorate open generics in Scrutor as well?

But as mentioned, most people think of AOP as magic stuff involving generating proxies and implementing interceptors. I have a fair bit of experience with how much magic it takes since I've written an interception library (LightInject.Interception). The difference is that the "decorator" is implemented at runtime (proxy) and the method calls are forwarded to the interceptor. The downside of this is that it is in many cases difficult to debug and to grasp for new developers. We will also take a performance hit since all method arguments are passed to the interceptor as object meaning there will be a lot of casting and boxing going on.

[dadhi]

The Decorators in DryIoc described here: https://github.com/dadhi/DryIoc/blob/master/docs/DryIoc.Docs/Decorators.md

In general the feature is similar to LightInject and combining with other library features may enable more things.

The important point is that Decorator registrations are distinguished from the "normal" Service registrations. This info is preserved by container and used to enable nesting, special diagnostics, control the caching, etc.

For the whole picture DryIoc has a 3rd kind of registrations - Wrappers, e.g. Func, Lazy, collections go here.

[ipjohnson]

So for external containers the decorator will be created by the external container and the decorated type (original registration) will be created using ActivatorUtilities.GetServiceOrCreateInstance(provider, type); not the external container?

Can you chain together decorators? Does it support more advanced scenarios like where the decorator can have Func<T> as the constructor parameter instead of just T?

[khellang]

So for external containers the decorator will be created by the external container and the decorated type (original registration) will be created using ActivatorUtilities.GetServiceOrCreateInstance(provider, type); not the external container?

Correct. The decorated type (decoratee) won't be resolved from the external container, but all its dependencies will.

This is also problematic for disposal, since the decorated instance won't be tracked by the container for disposal 😞

[tillig]

While I like the notion that it doesn't require any container authors to change anything, it also might be nice to allow containers that already implement decorators to do so natively. Similar to the way ISupportRequiredService allowed optional support for the GetRequiredService extension.

I'm also having a tough time working out mentally: What does it mean if some decorators are registered with these extension methods while others may be registered via the underlying container? For example:

public void ConfigureServices(IServiceCollection services)
{
  services.AddSingleton<IFoo, Foo>();
  services.Decorate<IFoo, Foo2>();
}

public void ConfigureContainer(ContainerBuilder builder)
{
  builder.RegisterDecorator<LoggingDecorator, IFoo>();
}

At least in Autofac, there's a whole resolution context around decoration so folks can intercept when something is getting decorated and do some complex things. I'm not clear how that would interact by some decoration being handled externally and some internally, like if there'd be some weirdness around things getting double decorated or possibly decorated in the wrong order.

I think if there was some way to map the IServiceCollection extensions to something native for those containers that support native decoration, it might be easier to address.

[ENikS]

Correct. The decorated type (decoratee) won't be resolved from the external container, but all its dependencies will.

This is a problem. My concerns are:

• How to keep track of the dependency graphs?
• Using containers configured outside of IServiceCollection
• Recurrent references?

[khellang]

Recurrent references?

The entire reason this is done is because of recurrent references (if I understand the term correctly). Decorators always get an instance of "itself" injected, i.e. you have an ICommandHandler<TCommand> that takes an ICommandHandler<TCommand> to decorate. This results in a StackOverflowException unless you do it this way.

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

[ah1508]

Hi,

The problem with the decorator pattern is that the decorator class must exist in the source, and it must implement the same interfaces as the decorated class. So for each decorated class, a decorator class is needed. Actually if several decorated classes implement the same interface then only one decorator class is needed for them, but a real class is still needed in the source.

Proxy pattern is better for cross cutting concerns (transaction, trace, etc...). Like for the decorator pattern, one proxy instance is created for each target instance but it is generated at runtime, no additional class is needed.

For example :

public interface IFoo
{
    void DoSomething();
}
public class Foo : IFoo
{
    public override void DoSomething()
    {
        // code
    }
}

public interface IBar
{
    void DoSomethingElse();
}
public class Bar : IBar
{
    public override void DoSomethingElse()
    {
        // code
    }
}

To apply transaction around method invocation :

public class Proxy : System.Reflection.DispatchProxy
{
    private object target;

    protected override object? Invoke(MethodInfo? targetMethod, object?[]? args)
    {
        using(var tx = new TransactionScope())
        {
            object ret = targetMethod.Invoke(target, args);
            tx.Complete();
            return ret;
        }
    }

    public static T For<T>(T target)
    {
        object proxy = System.Reflection.DispatchProxy.Create<T, Proxy>();
        ((Proxy)proxy).target = target;
        return (T)proxy;
    }
}

Service collection registration :

IServiceCollection sc = new ServiceCollection();
sc.AddSingleton<IFoo>(Proxy.For<IFoo>(new Foo());
sc.AddSingleton<IBar>(Proxy.For<IBar>(new Bar());

Suggested in #45760 :

IServiceCollection sc = new ServiceCollection();
sc.AddSingleton<IFoo, Foo>(fooInstance => Proxy.For<IFoo>(fooInstance));
sc.AddSingleton<IBar, Bar>(barInstance => Proxy.For<IBar>(barInstance));

The idea is to be involved in the service registration, a Func<TService, TService> receives the instance created by the default factory and returns something built from it (here : a proxy). What is returned replaces in the service collection the instance received as argument.

One limitation : the DispatchProxy is a interface based proxy so

• the target class must implement an interface.
• proxy is Foo or proxy is Bar return false
• only one interface can be declared in the Create method, what if the target class implements several interfaces ?

A inheritance based proxy would be much better.

With appropriate custom attributes ([Transactional], [Traceable], etc...) the proxy can adapt its interception logic to the target method. Example :

public class Foo : IFoo
{
    [Transactional]
    public override void Bar()
    {
        // code
    }
}

Better design : the proxy would apply a chain of interceptors (transactional interceptor, trace interceptor, etc...), the chain is based on the custom attributes discovered on target's methods. No custom attribute on any method for a given class means that there is no need for a proxy. This introspection has to be done only once.

But a developer should not have to create its own proxies and interceptors for common use cases...

This interception support should be built-in : built-in custom attributes for common use cases (transaction, trace....),built-in proxy creation with a chain of interceptors and the possibility to create custom interception logic. It has been a proven solution in Java for 10 years.

It would be similar to Aspnet ActionFilter but for any service registered in the IServiceCollection. It would benefit to custom interceptors, third party interceptors, microsoft interceptors. Something like that :

public interface IInterceptorAttribute: System.Attribute
{
    Object AroundInvoke(InvocationContext ctx);
}

public class TransactionalAttribute : IInterceptorAttribute
{
    public override Object AroundInvoke(InvocationContext ctx)
   {
        // do something before
        object ret = ctx.Proceed();
        // do something after
        return ret;
   }
}

With an async version of course.

[davidfowl]

This isn't something we would build into the default container. The bar is really high for new features, especially ones like this. I'd look at other containers that may have this built in

[Tiberriver256]

Might be a relevant addition to the conversation. This is a super simple method for a decorate extension that is borrowed from how HttpClient and HttpClientFactory work.

public static class ServiceCollectionExtensions {

  public static void Decorate<TInterface, TDecorator>(this IServiceCollection services)
    where TInterface : class
    where TDecorator : class, TInterface
  {
    // grab the existing registration
    var wrappedDescriptor = services.FirstOrDefault(
      s => s.ServiceType == typeof(TInterface));

    // check it&#039;s valid
    if (wrappedDescriptor == null)
      throw new InvalidOperationException($"{typeof(TInterface).Name} is not registered");

    // create the object factory for our decorator type,
    // specifying that we will supply TInterface explicitly
    var objectFactory = ActivatorUtilities.CreateFactory(
      typeof(TDecorator),
      new[] { typeof(TInterface) });

    // replace the existing registration with one
    // that passes an instance of the existing registration
    // to the object factory for the decorator
    services.Replace(ServiceDescriptor.Describe(
      typeof(TInterface),
      s => (TInterface)objectFactory(s, new[] { s.CreateInstance(wrappedDescriptor) }),
      wrappedDescriptor.Lifetime)
    );
  }

  private static object CreateInstance(this IServiceProvider services, ServiceDescriptor descriptor)
  {
    if (descriptor.ImplementationInstance != null)
      return descriptor.ImplementationInstance;

    if (descriptor.ImplementationFactory != null)
      return descriptor.ImplementationFactory(services);

    return ActivatorUtilities.GetServiceOrCreateInstance(services, descriptor.ImplementationType);
  }
}

Borrowed from this blog which goes into further detail: https://greatrexpectations.com/2018/10/25/decorators-in-net-core-with-dependency-injection

Things it does: ✅ Decorator can construct on an interface ✅ Wrapped implementation can be registered against the interface ✅ Decorator scope is taken from wrapped service ✅ Dependencies in wrapped implementation are injected automatically ✅ Dependencies in decorator are injected automatically

[Timovzl]

@Tiberriver256, it's good to see that we have nearly the same implementation.

I have the following suggestions:

• Use the last (i.e. winning) previous registration, rather than the first.
• Add rather than Replace the prior registration. After all, we cannot speak of the prior registration, but only of a prior registration; there may be multiple prior registrations, and replacing them all makes no sense. As such, I reason that replacing a single one makes no sense either.
• Support an overload with a custom factory parameter: Func<IServiceProvider, TInterface, TDecorator> decoratorFactory.
• Rename TInterface to TService, for consistency with existing methods.
• s.CreateInstance(wrappedDescriptor): I don't see any IServiceProvider.CreateInstance. This should probably just use GetRequiredService.

Although the above code and the suggestions are probably clear, I've included my own extension for reference. (It fulfills all the above, including the checkboxes.)

/// <summary>
/// Helps register decorator implementations that wrap existing ones in the container.
/// </summary>
internal static class DecoratorRegistrationExtensions
{
    /// <summary>
    /// Registers a <typeparamref name="TService"/> decorator on top of the previous registration of that type.
    /// </summary>
    /// <param name="lifetime">If no lifetime is provided, the lifetime of the previous registration is used.</param>
    public static IServiceCollection AddDecorator<TService, TImplementation>(
        this IServiceCollection services,
        ServiceLifetime? lifetime = null)
        where TService : class
        where TImplementation : TService
    {
        var decoratorFactory = ActivatorUtilities.CreateFactory(typeof(TImplementation),
            new[] { typeof(TService) });

        return AddDecorator<TService>(
            services,
            (serviceProvider, decoratedInstance) =>
                (TService)decoratorFactory(serviceProvider, new object[] { decoratedInstance }),
            lifetime);
    }

    /// <summary>
    /// Registers a <typeparamref name="TService"/> decorator on top of the previous registration of that type.
    /// </summary>
    /// <param name="decoratorFactory">Constructs a new instance based on the the instance to decorate and the <see cref="IServiceProvider"/>.</param>
    /// <param name="lifetime">If no lifetime is provided, the lifetime of the previous registration is used.</param>
    public static IServiceCollection AddDecorator<TService>(
        this IServiceCollection services,
        Func<IServiceProvider, TService, TService> decoratorFactory,
        ServiceLifetime? lifetime = null)
        where TService : class
    {
        // By convention, the last registration wins
        var previousRegistration = services.LastOrDefault(
            descriptor => descriptor.ServiceType == typeof(TService));

        if (previousRegistration is null)
            throw new InvalidOperationException($"Tried to register a decorator for type {typeof(TService).Name} when no such type was registered.");

        // Get a factory to produce the original implementation
        var decoratedServiceFactory = previousRegistration.ImplementationFactory;
        if (decoratedServiceFactory is null && previousRegistration.ImplementationInstance != null)
            decoratedServiceFactory = _ => previousRegistration.ImplementationInstance;
        if (decoratedServiceFactory is null && previousRegistration.ImplementationType != null)
            decoratedServiceFactory = serviceProvider => ActivatorUtilities.CreateInstance(
                serviceProvider, previousRegistration.ImplementationType, Array.Empty<object>());

        if (decoratedServiceFactory is null) // Should be impossible
            throw new Exception($"Tried to register a decorator for type {typeof(TService).Name}, but the registration being wrapped specified no implementation at all.");

        var registration = new ServiceDescriptor(
            typeof(TService), CreateDecorator, lifetime ?? previousRegistration.Lifetime);

        services.Add(registration);

        return services;

        // Local function that creates the decorator instance
        TService CreateDecorator(IServiceProvider serviceProvider)
        {
            var decoratedInstance = (TService)decoratedServiceFactory(serviceProvider);
            var decorator = decoratorFactory(serviceProvider, decoratedInstance);
            return decorator;
        }
    }
}

[WeihanLi]

Any update on this? Would this come in .NET 7?

[davidfowl]

The milestone is marked as "future", so no, this is not happening for .NET 7.

[WeihanLi]

@davidfowl thanks

[tomeyerman]

I see lots of code snippets here :) which is the recommended implementation until this is officially supported?

[khellang]

Scrutor has about 65M downloads in the meantime 😅