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.22k stars 152 forks source link

What is the difference between a root (concrete type) registration and a decorator, same type, both singleton? #855

Closed gitcob closed 4 years ago

gitcob commented 4 years ago

While writing unit tests I needed access to one of my decorators, which is registered with a singleton lifestyle, so I also registered it directly, also singleton lifestyle. I got some weird behavior, which I finally traced down to there being two instances in the container of the same class. I couldn't find anything regarding this behavior here or in the docs, so I boiled the problem down as much as possible to create an example where this happens:

public interface IMyInterface { }

public class TerminalClass : IMyInterface { }

public class Decorator : IMyInterface
{
    public static int CreatedCount;

    public Decorator(IMyInterface decorated) { CreatedCount++; }
}

[Fact]
public void SingletonRootAndDecorator()
{
    using var container = new Container();

    container.Register<IMyInterface, TerminalClass>(Lifestyle.Singleton);
    container.RegisterDecorator<IMyInterface, Decorator>(Lifestyle.Singleton);
    container.Register<Decorator>(Lifestyle.Singleton);

    container.Verify(VerificationOption.VerifyAndDiagnose);

    Assert.Equal(2, Decorator.CreatedCount);
    //Assert.Equal(1, Decorator.CreatedCount); // What I was expecting
}

I'm not sure I understand the decorator semantics here. If I change the lifestyle of one of the Decorator registrations, I get a DiagnosticVerificationException with a lifestyle mismatch, implying that SimpleInjector indeed wants to track a single instance of Decorator, so the lifestyle must be the same everywhere. Verification also does not see the two registrations as an issue. But there is still two of them.

I can probably work around this, but I'm curious if I'm fundamentally misunderstanding something.

dotnetjunkie commented 4 years ago

The RegisterDecorator method works very different compared to a 'normal' Register call. RegisterDecorator intercepts the original call by hooking into Simple Injector's ExpressionBuilt event. This allows the decorator () to be wrapped around the original built expression (the TerminalClass in your case).

When it comes to registering the Decorator class both using RegisterDecorator<IMyInterface, Decorator> and Register<Decorator>, Simple Injector doesn't relate these two registrations. What Simple Injector is concerned Register<Decorator> is a normal registration. And in this case, from Simple Injector's perspective, Decorator is not a decorator at all.

But this causes the mismatch. As Register<Decorator> is a registration on its own, Simple Injector will not try to deduplicate it, while it normally do in other cases. With the following code, for instance, Simple Injector will ensure only one instance of Foo is created:

container.Register<IFoo, Foo>(Lifestyle.Singleton);
container.Register<Foo>(Lifestyle.Singleton);

You might consider the behavior, where the decorator's registration is not de-duplicated, a design flaw. This is actually not a use case I think I have ever considered, which is why this has slipped through. Implementing it, however, might have ramifications that I can not yet oversee, and it might actually be pretty hard to fix. From a maintainers perspective, it might not be worthwhile the effort.

Do note though that there are other use cases where singleton decorators are created more than once, because even if they are singleton, there must be one instance per dependency. For instance:

container.Collection.Append<ILogger, ConsoleLogger>(Lifestyle.Singleton);
container.Collection.Append<ILogger, FileLogger>(Lifestyle.Singleton);
container.RegisterDecorator<ILogger, LoggerDecorator>(Lifestyle.Singleton);

In this case, each ILogger implementation gets wrapped with its own LoggerDecorator instance, even though it is registered as singleton. Compared to your use case, however, in this case the behavior is more obvious, because how do you create 1 single LoggerDecorator while it must depend on two different dependencies; you can't.

But long story short, this behavior is sort of "by design" and not easily fixed. But there are other ways to get hold of the decorators. What's the best approach, however, depends a bit on your use case. Can you elaborate on what it is you're trying to accomplish. I'm likely able to guide you towards a solution that is convenient and pleasant.

gitcob commented 4 years ago

Thanks for the detailed explanation, as always.

My use case involves an event publisher/handler system. The event handling part consists of a class that can be decorated and thus can support different optional features such as "fire and forget" and add an async scope (how to do that you explained yesterday, and it worked great). For properly testing the "fire and forget" feature I needed something that would let me await the event actually being handled in the unit test (basically the unit test has to circumvent the "forget" part), so my quick solution at the time was to add another decorator built around BufferBlock. To access that I figured I could register it with Register<Decorator> .

It's not a big deal, I think adding a decorator to a component that I was testing just for the test case itself was probably a bad idea, and I could just put the "barrier" into the actual event handler itself, since that has to be a mock anyway. Usually I treat decorators as pure functions, which is why I didn't come across this either.