jbogard / MediatR

Simple, unambitious mediator implementation in .NET
Apache License 2.0
11.2k stars 1.18k forks source link

Handlers for interfaced notifications are not working as expected #1050

Open Kjelli opened 4 months ago

Kjelli commented 4 months ago

The issue at hand

When using interfaced notifications, and handlers for the interfaces - the handler does not run unless there exists a handler targetting a concrete implementation of the respective interface.

Steps to reproduce:

Bootstrap

HostApplicationBuilder builder = Host.CreateApplicationBuilder(args);
builder.Services.AddMediatR(c => c.RegisterServicesFromAssembly(typeof(Program).Assembly));
using IHost host = builder.Build();

var mediator = host.Services.GetRequiredService<IMediator>();

A) A simple notification handler that runs

// Simple case, nothing new

record SimpleGreeterNotification(string Message) : INotification;
class SimpleHandler : INotificationHandler<SimpleGreeterNotification>
{
    public Task Handle(SimpleGreeterNotification notification, CancellationToken cancellationToken)
    {
        Console.WriteLine(notification.Message);
        return Task.CompletedTask;
    }
}

// ...

await mediator.Publish(new SimpleGreeterNotification("Hello")); 
// Outputs "Hello", no surprises here

B) An interfaced notification handler that runs

// Interfaced case

public interface IGreeterInterface : INotification;
record ImplementingGreeterNotification(string Message) : IGreeterInterface;

class InterfacedGreeterHandler : INotificationHandler<IGreeterInterface>
{
    public Task Handle(IGreeterInterface notification, CancellationToken cancellationToken)
    {
        Console.WriteLine("I only run when ImplementingGreeterHandler exists");
        return Task.CompletedTask;
    }
}

class ImplementingGreeterHandler : INotificationHandler<ImplementingGreeterNotification>
{
    public Task Handle(ImplementingGreeterNotification notification, CancellationToken cancellationToken)
    {
        Console.WriteLine(notification.Message);
        return Task.CompletedTask;
    }
}

// ...

await mediator.Publish(new ImplementingGreeterNotification("Hello interface"));
// Outputs "I only run when ImplementingGreeterHandler exists"
// Outputs "Hello interface

C) Now for the interfaced notification handler that does not run

// Interfaced case

public interface IGreeterInterface : INotification;
record ImplementingGreeterNotification(string Message) : IGreeterInterface;

class InterfacedGreeterHandler : INotificationHandler<IGreeterInterface>
{
    public Task Handle(IGreeterInterface notification, CancellationToken cancellationToken)
    {
        Console.WriteLine("I only run when ImplementingGreeterHandler exists");
        return Task.CompletedTask;
    }
}

// ...

await mediator.Publish(new ImplementingGreeterNotification("Hello interface"));
// No output

Cases A and B are as expected, whereas case C is strange - I would expect InterfacedGreeterHandler to be run.

Further notes

I am not sure any of this is relevant, but I did some investigating.

I have a theory that this issue is caused near NotificationHandlerWrapper. Inspecting the service collection, I verify that it correctly contains INotificationHandler<IGreeterInterface>, but it does not contain INotificationHandler<ImplementingGreeterNotification>.

From these lines I see that IServiceProvider.GetServices<INotificationHandler<ImplementingGreeterNotification>> does not return anything, which I suspect subsequently fails to resolve a respective NotificationHandlerExecutor. https://github.com/jbogard/MediatR/blob/cac76bee59a0f61c99554d76d4c39d67810fb406/src/MediatR/Wrappers/NotificationHandlerWrapper.cs#L24-L26

It does make sense I suppose, but at face value this looks like odd behavior either way.

I tested a more basic, yet potentially related case; implementing a catch-all INotificationHandler<INotification> never runs if there are no other INotificationHandler-types registered.

github-actions[bot] commented 2 months ago

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

Kjelli commented 2 months ago

bump

richardpauly commented 2 months ago

I seem to have the same problem in my project

Robert-LTH commented 2 months ago

I stumpled upon this issue as well but solved it by copying the definition of the handler from https://github.com/jbogard/MediatR.Extensions.Microsoft.DependencyInjection/blob/020d8817ae84e18d61bbf343b00558249c37f899/src/TestApp/ConstrainedPingedHandler.cs#L8

inkvizytor commented 1 month ago

I stumpled upon this issue as well but solved it by copying the definition of the handler from https://github.com/jbogard/MediatR.Extensions.Microsoft.DependencyInjection/blob/020d8817ae84e18d61bbf343b00558249c37f899/src/TestApp/ConstrainedPingedHandler.cs#L8

It does not solve case C.

adam-dot-cohen commented 1 month ago

It's been over a year since I last wrestled with notifications. I threw in the towel and pursued an alternative approach described with pseudo code in this thread last year. Drastically simplified notifications and cloud events and made execution order and debugging possible. Unfortunately, it seems the saga continues and I no longer have access to the code base. In case it helps anyone, the suggested alternative at the bottom of this thread worked like a charm...

How to change MediatR publish strategy