jbogard / MediatR

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

[Duplicate] Notification Handler incorrectly executing twice #948

Closed Mayron closed 11 months ago

Mayron commented 11 months ago

I have created an ASP.Net Core Web API with tests to demonstrate and recreate the bug: https://github.com/Mayron/Demo-NotificationService/tree/master

The README explains most of the issue in detail, but to summarise:

When any notification handler implementing the INotificationHandler interface exists in the codebase that makes use of a generic type parameter, then it causes all other notification handlers to incorrectly be executing twice for each mediator publish call.

For example:

// Accepts multiple subclass types of AuditLogNotification and works fine.
public class AuditLogNotificationHandler<T> : INotificationHandler<T> where T : AuditLogNotification

// This one does not have a generic type argument but due to the other handler existing, it gets executed twice instead of once
public class NewUserNotificationHandler : INotificationHandler<NewUserNotification>

This shows how I'm publishing the events, but please view my git repository for the full code and README for more details:

// BUG: This gets executed twice
await _mediator.Publish(new NewUserNotification(newUser, accountId));

// These all get executed once as expected.
await _mediator.Publish(new UserAuditLogNotification("New User Created", newUser.Id));
await _mediator.Publish(new AccountAuditLogNotification("User Joined Account", accountId));
await _mediator.Publish(new AuditLogNotification("User Created", true));

This only seems to happen if the service provider's ValidateOnBuild configuration option is set to true to run a useful service dependency validation check during the build process (which our dev team rely on, but we have temporarily disabled it).

I believe this is due to this validation step creating 2 service descriptors for each type of class that implements INotificationHandler instead of 1 (one being an array of that service, and 1 being the service itself) because the generic type constraint confuses the process.

Perhaps the AddMediatR extension method could accommodate this validation check by tweaking the registered services to prevent this bug. Just a theory and it needs more investigation.

Mayron commented 11 months ago

This might be a duplicate of https://github.com/jbogard/MediatR/issues/884 but I'm not sure if my scenario is the same.

I ended up making the repository to isolate my issue as part of a work investigation task I was assigned.

I thought I'd share it here as a new issue and link it to that existing one in case it helps.

Mayron commented 11 months ago

I'm pretty sure this issue explains the problem perfectly: https://github.com/dotnet/runtime/issues/87017

But unfortunately, my project uses an older version of .NET Core so I'm hoping a MediatR update can provide a workaround for this bug so I only have to update MediatR. It sounds like this is being actively worked on according to the comment in the other MediatR issue.

Feel free to close this issue if this is the case, but please feel free to use my repo for testing or investigating if it helps in anyway.

Thanks all for the great work!

jbogard commented 11 months ago

I don't try to work around issues in the stock MS DI container. Instead you should investigate 3rd party containers (Lamar, Autofac etc.)