simpleinjector / SimpleInjector

An easy, flexible, and fast Dependency Injection library that promotes best practice to steer developers towards the pit of success.
https://simpleinjector.org
MIT License
1.21k stars 155 forks source link

Add DependencyMetadata<T> support for decorators #879

Open dotnetjunkie opened 3 years ago

dotnetjunkie commented 3 years ago

Simple Injector supports injecting metadata into decorators for a long time through the use of DecoratorContext. More recently (v5) metadata support has been added more broadly through the use of DependencyMetadata<T>. This, however, means that there are now two mechanisms to inject metadata. The decorator subsystem, unfortunately, doesn't support getting DependencyMetadata<T> injected which can be confusing, as reported in #878.

RyanMarcotte commented 3 years ago

I'll note that the injection of DependencyMetadata<T> in a decorator does work and behaves as expected in the following scenario:

public class DecoratorWithDependencyMetadataTests
{
    [Fact]
    public void SuccessfullyAppliesDecoratorWithDependencyMetadataToType()
    {
        var container = new Container();
        container.Register<IDoTheThing, Thing>();
        container.RegisterDecorator<IDoTheThing, Decorator1>();
        container.RegisterDecorator<IDoTheThing, Decorator2>();

        container.Verify(); // no exception thrown

        container.GetInstance<IDoTheThing>().DoTheThing();
    }

    private interface IDoTheThing
    {
        void DoTheThing();
    }

    private class Thing : IDoTheThing
    {
        public void DoTheThing() { }
    }

    private class Decorator1 : IDoTheThing
    {
        private readonly IDoTheThing _thing;
        private readonly DependencyMetadata<IDoTheThing> _dependencyMetadata;

        public Decorator1(IDoTheThing thing, DependencyMetadata<IDoTheThing> dependencyMetadata)
        {
            _thing = thing;
            _dependencyMetadata = dependencyMetadata;
        }

        public void DoTheThing() => _thing.DoTheThing();
    }

    private class Decorator2 : IDoTheThing
    {
        private readonly IDoTheThing _thing;
        private readonly DependencyMetadata<IDoTheThing> _dependencyMetadata;

        public Decorator2(IDoTheThing thing, DependencyMetadata<IDoTheThing> dependencyMetadata)
        {
            _thing = thing;
            _dependencyMetadata = dependencyMetadata;
        }

        public void DoTheThing() => _thing.DoTheThing();
    }
}
dotnetjunkie commented 3 years ago

@RyanMarcotte, that's peculiar. This behavior might be more accidental than intentional, because there are currently no unit tests to support that scenario. You might want to be careful with that until it is officially supported (and thoroughly tested).

RyanMarcotte commented 3 years ago

For sure. I will use DecoratorContext for my particular scenario as you suggested in #878 . Thanks again!

RyanMarcotte commented 3 years ago

Instead of adding support for DependencyMetadata<T> in decorators... Might it be easier / make more sense to throw an exception that directs people to use DecoratorContext instead of DependencyMetadata<T>? My impression is that DecoratorContext is better suited than DependencyMetadata<T> for these scenarios and - if so - developers should be pushed to best practice. For example, I can use DecoratorContext to retrieve the collection of decorators being applied to the service type.

dotnetjunkie commented 3 years ago

@RyanMarcotte, good point. I will take this into consideration.

RyanMarcotte commented 3 years ago

I'd be happy to contribute a pull request once the desired behavior is finalized. Let me know!