Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
Applying fix
  • Loading branch information
jbogard committed Feb 22, 2026
commit e1f6caaa2cfbe28eeebe896e79a01f061712df49
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ internal static void CheckLicense(this IServiceProvider serviceProvider)
serviceProvider.GetRequiredService<MediatRServiceConfiguration>(),
serviceProvider.GetRequiredService<ILoggerFactory>()
);
var licenseValidator = serviceProvider.GetService<LicenseValidator>()
var licenseValidator = serviceProvider.GetService<LicenseValidator>()
?? new LicenseValidator(serviceProvider.GetRequiredService<ILoggerFactory>());
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PR description states that this method should be simplified to use GetRequiredService for both licenseAccessor and licenseValidator, removing the fallback logic with ?? new LicenseAccessor(...) and ?? new LicenseValidator(...). However, this change was not implemented - only a trailing whitespace was removed from line 69. The fallback logic on lines 65-70 should be replaced with: var licenseAccessor = serviceProvider.GetRequiredService<LicenseAccessor>(); and var licenseValidator = serviceProvider.GetRequiredService<LicenseValidator>(); as described in the PR description.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot open a new pull request to apply changes based on this feedback


var license = licenseAccessor.Current;
Expand Down
22 changes: 20 additions & 2 deletions src/MediatR/Registration/ServiceRegistrar.cs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
using MediatR.Pipeline;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.DependencyInjection.Extensions;
using Microsoft.Extensions.Logging;

namespace MediatR.Registration;

Expand Down Expand Up @@ -470,8 +471,25 @@ public static void AddRequiredServices(IServiceCollection services, MediatRServi
MediatRServiceCollectionExtensions.LicenseChecked = false;

services.TryAddSingleton(serviceConfiguration);
services.TryAddSingleton<LicenseAccessor>();
services.TryAddSingleton<LicenseValidator>();
services.TryAddSingleton(static sp =>
{
var loggerFactory = sp.GetService<ILoggerFactory>()
?? throw new InvalidOperationException(
"MediatR requires ILoggerFactory to be registered. " +
"Call services.AddLogging() before services.AddMediatR().");
Comment on lines +476 to +479
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

New behavior is being introduced here (custom InvalidOperationException when ILoggerFactory is missing). There isn’t currently a test asserting this specific failure mode/message (the suite generally calls AddFakeLogging). Adding a focused test that omits logging and asserts the thrown message would help prevent regressions back to the cryptic "No constructor" error.

Copilot uses AI. Check for mistakes.
Comment on lines +476 to +479
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The exception message says to call services.AddLogging() before services.AddMediatR(), but registration order doesn’t actually matter as long as ILoggerFactory is registered before the provider is built/resolution happens (e.g., tests call AddRequiredServices before AddFakeLogging). Consider rewording to avoid implying a required ordering (e.g., “Ensure ILoggerFactory is registered (services.AddLogging())”).

Copilot uses AI. Check for mistakes.
var config = sp.GetService<MediatRServiceConfiguration>();
return config != null
? new LicenseAccessor(config, loggerFactory)
: new LicenseAccessor(loggerFactory);
Comment on lines +480 to +483
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MediatRServiceConfiguration is always registered just above via services.TryAddSingleton(serviceConfiguration), so sp.GetService<MediatRServiceConfiguration>() should never be null here. This makes the conditional and the new LicenseAccessor(loggerFactory) branch effectively dead code; consider switching to GetRequiredService<MediatRServiceConfiguration>() and always using the (config, loggerFactory) constructor to simplify.

Suggested change
var config = sp.GetService<MediatRServiceConfiguration>();
return config != null
? new LicenseAccessor(config, loggerFactory)
: new LicenseAccessor(loggerFactory);
var config = sp.GetRequiredService<MediatRServiceConfiguration>();
return new LicenseAccessor(config, loggerFactory);

Copilot uses AI. Check for mistakes.
});
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TryAddSingleton call is missing the type parameter. This will not register LicenseAccessor in the DI container correctly. Change line 474 to: services.TryAddSingleton<LicenseAccessor>(static sp =>

Copilot uses AI. Check for mistakes.
services.TryAddSingleton(static sp =>
{
var loggerFactory = sp.GetService<ILoggerFactory>()
?? throw new InvalidOperationException(
"MediatR requires ILoggerFactory to be registered. " +
"Call services.AddLogging() before services.AddMediatR().");
return new LicenseValidator(loggerFactory);
});
Copy link

Copilot AI Feb 22, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The TryAddSingleton call is missing the type parameter. This will not register LicenseValidator in the DI container correctly. Change line 485 to: services.TryAddSingleton<LicenseValidator>(static sp =>

Copilot uses AI. Check for mistakes.

var notificationPublisherServiceDescriptor = serviceConfiguration.NotificationPublisherType != null
? new ServiceDescriptor(typeof(INotificationPublisher), serviceConfiguration.NotificationPublisherType, serviceConfiguration.Lifetime)
Expand Down