Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add DependencyMetadata<T> support for decorators #879

Open
dotnetjunkie opened this issue Jan 6, 2021 · 6 comments
Open

Add DependencyMetadata<T> support for decorators #879

dotnetjunkie opened this issue Jan 6, 2021 · 6 comments
Labels
Milestone

Comments

@dotnetjunkie
Copy link
Collaborator

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
Copy link

RyanMarcotte commented Jan 6, 2021

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
Copy link
Collaborator Author

@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
Copy link

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

@RyanMarcotte
Copy link

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
Copy link
Collaborator Author

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

@RyanMarcotte
Copy link

RyanMarcotte commented Jan 6, 2021

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

@dotnetjunkie dotnetjunkie added this to the Simple Injector v6.0 milestone May 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants