DefaultServiceLifetime.Transient registers Mediator as transient too as opposed to ReadMe

[gfoidl]

ReadMe states

• Transient - handlers registered as transient, IMediator/Mediator/ISender/IPublisher still singleton

source

but I see the code generated as Transient for the Mediator too (version 2.1.7):

services.Add(new SD(typeof(global::Mediator.Mediator), typeof(global::Mediator.Mediator), global::Microsoft.Extensions.DependencyInjection.ServiceLifetime.Transient));
services.TryAdd(new SD(typeof(global::Mediator.IMediator), sp => sp.GetRequiredService<global::Mediator.Mediator>(), global::Microsoft.Extensions.DependencyInjection.ServiceLifetime.Transient));
services.TryAdd(new SD(typeof(global::Mediator.ISender), sp => sp.GetRequiredService<global::Mediator.Mediator>(), global::Microsoft.Extensions.DependencyInjection.ServiceLifetime.Transient));
services.TryAdd(new SD(typeof(global::Mediator.IPublisher), sp => sp.GetRequiredService<global::Mediator.Mediator>(), global::Microsoft.Extensions.DependencyInjection.ServiceLifetime.Transient));

As DefaultServiceLifetime -> ServiceLifetime in #24 is that a documentation issue (i.e. need to update the ReadMe -- I can send a PR) and / or is the behavior by intention?

I'd really like to have the Mediator registered as singleton, while the handlers, etc. be transient / scoped.

[TimothyMakkison]

I'd really like to have the Mediator registered as singleton, while the handlers, etc. be transient / scoped

Not sure what the expected behaviour should be, but for the time being you could you preemptively add the services before calling AddMediator. TryAdd will prevent the re-registration of the types.

[gfoidl]

Thanks for the reminder (sometimes I forget the easy workarounds)!
Just to re-add the Mediator after AddMediator again (that one isn't a TryAdd), so DI will use the last one registered.

Not sure what the expected behaviour should be

Maybe provide another option to configure that lifetime too? Something like

public sealed class MediatorOptions
{
    public string Namespace { get; set; } = "Mediator";

    public ServiceLifetime MediatorServiceLifetime { get; set; } = ServiceLifetime.Singleton;
    public ServiceLifetime HandlerServiceLifetime  { get; set; } = ServiceLifetime.Scoped;
}

Renaming the ServiceLifetime property (again) is a breaking-change. But as version 3 is under development this could be taken (also as the impact of that change isn't really big, because when updating from v2 -> v3 one has to only a few places to change -- due to source-generation it's not a binary breaking change anyway (?!)).

[martinothamar]

Hmm I think it used to be as documented atleast, maybe there was a bug at some point that required this. Don't remember but I'll try to dig into it

[martinothamar]

Oo yes apparantly this was a bug that was fixed: #73
I don't think there is any way around that. We could add the configuration option as you suggest but it is kind of a footgun isn't it? I'm not sure, what do you guys think?

I'll have to update the README in any case, thanks!

[gfoidl]

Thanks for finding the PR.

Would it be possible that instead of #73 for the activation a new servce cope is created for the handlers?
Then IMediator/Mediator/ISender/IPublisher could be singletons, whilst the transient / scoped registered handlers are resolved from the created service scope (which isn't the root-scope then)?

[martinothamar]

Right now, if configured as transient, but used/injected from ASP.NET Core, scoped instances of services could (depending on how IMediator is injected) be shared amongst Mediator handlers. If we made a new scope per Mediator invocation we would not really respect wether or not IMediator originates from a scope, if you get what I mean. That feels unexpected to me, and I would think I would get a bug report from that at some point.

Can I ask what is problematic about this behavior, is it the memory allocation? Or is initialization expensive? I honestly have not looked too much at perf for transient configuration apart from that one optimization which had to be removed

[gfoidl]

if you get what I mean

Yep, I understand, and that's a very valid point.

is it the memory allocation?

Yes, but TBH there are larger allocations (not from Mediator) that should be elided first. So more or less a micro-optimization.

For the scenario w/ ASP.NET Core's request there are some way to differentiate if it's the root scope or not (at simplest, just a reference compare), but now I believe that this adds more overhead and for sure complicates the SGen very much.

Thus the current (documented) behavior is fine for me.
When I have more time I'd like to dig into the code and play a bit around to try the (very rough) outline from the last paragraph, and see if that's a) reliable and b) shows up in profiles.