khellang / Scrutor

Assembly scanning and decoration extensions for Microsoft.Extensions.DependencyInjection
MIT License
3.58k stars 234 forks source link

Suggestion: Missing methods for decorate #77

Open mjebrahimi opened 5 years ago

mjebrahimi commented 5 years ago

It would be great if these methods had been added for Decorator DecorateTransient<>(); DecorateScoped<>(); DecorateSingleton<>();

Currently, both Decorator and the class which is decorating has been registered by the same LifeTime.

Assume that "MyService" class is registered as Singleton and we need to register "MyServiceDecorator" as Scoped. but currently, it's not possible.

I can make the changes and create a pull request if you want

khellang commented 5 years ago

As far as I can remember, this isn't really possible as we replace the existing descriptor with the decorated ones. That's how we're able to make decoration work at all. See https://github.com/khellang/Scrutor/issues/38. But by all means, give it a try. I'd love to be proven wrong :wink:

mjebrahimi commented 5 years ago

I read the whole discussion of #38 issue and I believe that this does not cause any problems and it is necessary to do so.

currently, if our decorator has dependencies with a shorter lifetime than itself, that cause captive dependency problem and fixing this problem is impossible because we can not change the lifetime of that decorator.

Basically, the captive dependency problem does not depend on the decorator. This problem occurs when a service has dependencies with a shorter lifetime than itself, even if the Decorator is not used. However, this is a common scenario, especially in decorators.

I think it's better to not change the behavior of Decorate method (to transient by default) to prevent breaking changes. But it's better to add the DecorateScoped, DecorateTransient methods to prevent the captive dependency problem.

It is also better to add the DecorateSingleton method to integrity and avoid confusing, although it does not necessarily, because the lifetime of the Decorator class must be equal or shorter than to the Decoratee lifetime, and if the Decoratee is singleton and we want to Decorator be singleton, there is not different between the Decorate method(default) with the DecorateSingleton.

Anyway, I reviewed the codes and tested this feature and I believe this is possible.

I'm sorry if my English is not good enough

khellang commented 5 years ago

Anyway, I reviewed the codes and tested this feature and I believe this is possible.

Awesome! Looking forward to the pull request 😎

mjebrahimi commented 5 years ago

Related PRs: #78, #79 and #80