De-duping notification handlers before dispatching#1159
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes issue #1118 where notification handlers were being called multiple times when using DI containers that support variance (e.g., DryIoc). When a derived notification type (E2 : E1) exists alongside handlers for both base (C1 : INotificationHandler) and derived types (C2 : INotificationHandler), publishing E2 would incorrectly invoke C1 twice due to the contravariant interface combined with MediatR's intentional dual registration for MSDI compatibility.
Changes:
- Added deduplication logic in
NotificationHandlerWrapper.csusingGroupBy(x => x.GetType())pattern to ensure each handler type is invoked only once - Added comprehensive regression tests in
Issue1118Tests.csto verify handlers are called exactly once for both base and derived events - Minor whitespace cleanup in
ServiceRegistrar.cs
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/MediatR/Wrappers/NotificationHandlerWrapper.cs |
Added deduplication logic using GroupBy/Select pattern to prevent duplicate handler invocations |
test/MediatR.Tests/MicrosoftExtensionsDI/Issue1118Tests.cs |
New regression tests verifying handlers are called exactly once for base and derived notification events |
src/MediatR/Registration/ServiceRegistrar.cs |
Removed trailing whitespace (formatting only) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This was referenced Mar 3, 2026
This was referenced Mar 8, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fix: Duplicate Notification Handler Invocation (Issue #1118)
Context
When a derived notification type (
E2 : E1) exists alongside a handler for the base type (C1 : INotificationHandler<E1>) and a handler for the derived type (C2 : INotificationHandler<E2>), publishingE2incorrectly callsC1twice with DI containers that support variance (e.g. DryIoc).Root cause:
INotificationHandler<in TNotification>is contravariant. MediatR'sServiceRegistrarregistersC1for bothINotificationHandler<E1>ANDINotificationHandler<E2>(viaCanBeCastTo/IsAssignableFrom). This is intentional — it allows MSDI (which does not support variance) to dispatchC1when publishingE2. However, DI containers that do support variance (like DryIoc) additionally returnC1from their own contravariant resolution ofINotificationHandler<E1>, yielding C1 twice.Why a registration-only fix doesn't work: Removing
C1'sINotificationHandler<E2>registration breaks MSDI users — C1 would never be called when publishing E2 with MSDI.Fix
Deduplicate by handler type in
NotificationHandlerWrapperImpl<TNotification>.Handle()before building the executor list. Each handler class is then invoked at most once, regardless of how many times the DI container returns it.src/MediatR/Wrappers/NotificationHandlerWrapper.cs:ServiceRegistrar.csis unchanged.Tests
test/MediatR.Tests/MicrosoftExtensionsDI/Issue1118Tests.cs(new file):Publishing_BaseEvent_Should_Call_BaseEventHandler_OncePublishing_DerivedEvent_Should_Call_BaseEventHandler_ExactlyOncePublishing_DerivedEvent_Should_Call_DerivedEventHandler_OnceHandlers use
IServiceProvider(always injectable) to soft-resolveCallLog, soPipelineTests(which usesValidateOnBuild=trueon the same assembly) does not fail.Verification