Allow user to specify DI scope for RequestHandlers

[DaveSenn]

I’m working on my first application (asp.net) using MediatR. Naively I’ve assumed that RequestHandlers will be scoped as singleton (which as I found out is not the case). The reason I want my handler to be a singleton is that I want to send a message to a message queue when my handler gets invoked. The idea is that multiple classes throughout my application can publish some requests and they will be sent to a message queue. Having to open a new connection for each message would be huge performance overhead.

While searching for a way to make my handler a singleton I found this issue: https://github.com/jbogard/MediatR.Extensions.Microsoft.DependencyInjection/issues/34

I did some local testing with your source and I’ve an idea how it would be possible to solve the lifetime issue.

First there is the following workaround:

public class MyRequestHandler : IRequestHandler<MyRequest> {}
…
var services = new ServiceCollection();
services.AddSingleton<IRequestHandler<MyRequest, Unit>,MyRequestHandler>();
services.AddMediatR( typeof(Program) );

Registering the handler as IRequestHandler<,> before calling AddMediatR works. MediatR does not replace the service in the service collection.

It is my understanding that all RequestHandlers (found in the specified assembly) are registered in ServiceRegistrar. ConnectImplementationsToTypesClosing and AddConcretionsThatCouldBeClosed. The current implementation uses TryAddTransient to register all handlers. The implementation could also use TryAddSingleton or TryAddScoped. Owever there is no way to configure which scope should be used for the handlers. One way of adding support for different scopes would be to add an optional attribute to a RequestHadnler implementation. A property on this attribute could be used to select the scope to which the handler should be added.

public class MediatRScope : Attribute
{
    public Scope Scope { get; }

    public MediatRScope(Scope scope)
    {
        Scope = scope;
    }
}

public enum Scope
{
    Singleton,
    Transient,
    Scoped
}

[MediatRScope(Scope.Singleton)]
public class MyRequestHandler : IRequestHandler<MyRequest> {}

Do you think this could be an option? I’ve so I would be happy to open a pull request.

[jbogard]

Typically you make your handlers transient, and their dependencies then have their unique lifecycle. In your case, why not introduce a dependency between handlers and mark that dependency singleton?

[DaveSenn]

@jbogard Why would I do that? The only thing I would get from creating new instances ever time is more work for the GC. Why don’t you want to support singletons in MediatR? In some cases a singleton is the best tool as are transient or scoped services in other cases. Why not giving the developers the freedom to choose whichever tool they want to use?

[lilasquared]

What else is the handler doing? If it is simply taking the request and writing it to a queue then I would say that a handler may be the wrong tool to use for that (just my opinion). I think a request / handler combination is geared more towards encapsulating high level user actions. It sounds like you can accomplish what you want outside of the mediator pattern altogether with a simple singleton message writer of some kind.

If the handler is doing more work and writing the message is just a small part of the work, then Single Responsibility Principle would say writing that message probably should be part of a different class anyway, which would just be the singleton dependency of the handler.

Another problem with singleton handlers is that transient dependencies and scoped dependencies don't get reconstructed due to the singleton parent, which can cause tricky bugs if you're not paying attention to your dependency graph.

Just my opinions from reading the thread and I use MediatR regularly.

[jbogard]

I don’t want to add a feature that I’ll then have to deal with all the bug reports because people don’t understand of dependency lifetimes work. Because when you make a service singleton, its entire dependency chain also needs to be singleton. I really don’t feel like having to explain that over and over again.

On Jul 15, 2020, at 3:41 PM, Aaron Roberts notifications@github.com wrote:

 What else is the handler doing? If it is simply taking the request and writing it to a queue then I would say that a handler may be the wrong tool to use for that (just my opinion). I think a request / handler combination is geared more towards encapsulating high level user actions. It sounds like you can accomplish what you want outside of the mediator pattern altogether with a simple singleton message writer of some kind.

If the handler is doing more work and writing the message is just a small part of the work, then Single Responsibility Principle would say writing that message probably should be part of a different class anyway, which would just be the singleton dependency of the handler.

Another problem with singleton handlers is that transient dependencies and scoped dependencies don't get reconstructed due to the singleton parent, which can cause tricky bugs if you're not paying attention to your dependency graph.

Just my opinions from reading the thread and I use MediatR regularly.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[DaveSenn]

@jbogard I can see your point with not wanting to deal with the bug reports. But on the other hand, a dependency tree is not something terribly complicated. (Btw. the dependency of a singleton must not be a singleton itself it can also be a transient (e.g. something with no shared state and no support for thread safe access) …just not a scoped service.) Do you think there would be a lot of bug reports/questions if the documentation would explain the scope issue?

[jbogard]

Yeah, it would.

Most folks use this library for ASP.NET Core, where there is a scope for each request. I don't want to go down the path of customizing individual handlers, transient is safest and gives me the fewest headaches.