jbogard / MediatR

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

MediatR + Autofac NotificationHandler decorator pattern strange behaviour. #1063

Closed GianlucaLocri closed 4 days ago

GianlucaLocri commented 2 months ago

Hi, I'm having a very strange issue with decorator pattern using MediatR (12.4.0) and Autofac (8.0.0) but I really cannot understand wether the problem is concerning Autofac, MediatR or (more probably) my code...

Basically I have a custom wrapper around INotification:

// My custom notification wrapping the event
public interface IEventNotification<TEvent> : INotification
    where TEvent : IEvent
{
    TEvent Event { get; init; }
}

that is handled by this interface

// The custom Notification Handler.
public interface IEventNotificationHandler<TEvent> : INotificationHandler<IEventNotification<TEvent>>
        where TEvent : IEvent
{
}

My application will (very simplified) raise events. These events (whose implementations are known) are handled by some handlers. In some cases there are multiple handlers per event.

Now my code is working as expected until I introduce a decorator for the event handlers like this one

public class CustomNotificationHandlerDecorator<TDomainEvent> : IEventNotificationHandler<TDomainEvent>
    where TDomainEvent : IEvent
{
    private readonly INotificationHandler<IEventNotification<TDomainEvent>> _decorated;

    public CustomNotificationHandlerDecorator(IEventNotificationHandler<TDomainEvent> decorated)
    {
        _decorated = decorated;
    }

    public Task Handle(IEventNotification<TDomainEvent> notification, CancellationToken cancellationToken)
    {
        Console.WriteLine("I am the custom decorator!");
        _decorated.Handle(notification, cancellationToken);
        Console.WriteLine("Decorator job completed\n");
        return Task.CompletedTask;
    }
}

At this point, for a given event, the decorator is applied only to one of the declared handlers and the handlers itself is called multiple times!

This is somewhat difficult to explain, so I crafted a fully functional but yet simple example on dotnetfiddle: https://dotnetfiddle.net/5sIGzj

As you can see, only the SecondEventNotificationHandler is called (and decorated) twice. If the line 155 builder.RegisterGenericDecorator(typeof(CustomNotificationHandlerDecorator<>),typeof(INotificationHandler<>)); is commented out, the two event handlers are called as expected, (but the decorator is obviously not called).

I am struggling with this for several days and I really cannot get my head around... Thanks for the support and for your amazing work!

crnd commented 2 months ago

Does the behavior change in any way if you downgrade MediatR to 12.2.0?

GianlucaLocri commented 2 months ago

Does the behavior change in any way if you downgrade MediatR to 12.2.0?

Unfortunately no. I've tried with 12.2.0 and also other few random previous version of both MediatR and Autofac without success.

GianlucaLocri commented 2 months ago

Today I found an issue on Autofac repo that started me thinking that the problem might be on the autofac side. https://github.com/autofac/Autofac/issues/1330

TL;DR; To my understanding, the problem is right here: https://github.com/jbogard/MediatR/blob/cac76bee59a0f61c99554d76d4c39d67810fb406/src/MediatR/Wrappers/NotificationHandlerWrapper.cs#L24-L26

MediatR is using an IServiceProvider to get all the handlers implementing INotificationHandler. In my case (see https://dotnetfiddle.net/5sIGzj) TNotification is IEventNotification<TestEvent>>.

In fact I verified that this line serviceFactory.GetServices<INotificationHandler<IEventNotification<TestEvent>>>(); indeed returns 2 decorators with the same decorated service inside (the same handler).

If instead I do not register the generic decorator, the above same line returns two distinct handlers as expected...

pnagoorkar commented 2 months ago

This appears to be an autofac caused behavior, although I wouldn't call it a problem.

Taking a look at your code, you're registering 3 types of INotificationHandler<IEventNotification> such that the final registration is asking for (without specificity) an instance of INotificationHandler<IEventNotification>.

What you're observing is a result of autofac defaulting to the final registration of the requested type.

If you swap (define first handler after second) the order in which your handlers are declared, your output will change and you will see 2 outputs from the first handler.

You may want to look at autofac's keyed services fearure to help you define which instance you want injected in your decorator.

GianlucaLocri commented 2 months ago

This appears to be an autofac caused behavior, although I wouldn't call it a problem.

Taking a look at your code, you're registering 3 types of INotificationHandler such that the final registration is asking for (without specificity) an instance of INotificationHandler.

What you're observing is a result of autofac defaulting to the final registration of the requested type.

If you swap (define first handler after second) the order in which your handlers are declared, your output will change and you will see 2 outputs from the first handler.

You may want to look at autofac's keyed services fearure to help you define which instance you want injected in your decorator.

Thanks for your reply though I cannot get how this is a desired (or expected) behaviour... I'm following the autofac decorator pattern from autofac specification. If I don't register the generic decorator, when MediatR ask to the service factory for all the instances of INotificationHandler<TNotification> then autofac returns the two registered handlers as expected.

However if the decorator is registered the decorated handler is not resolved correctly and the last registered instance is returned every time inside the decorator constructor. To my understanding, the decorator should be called for each individual un-decorated registration. Is there something I didn't get correctly?

github-actions[bot] commented 2 weeks 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.

github-actions[bot] commented 4 days ago

This issue was closed because it has been stalled for 14 days with no activity.