StartupValidator multiple registrations.

[PetSerAl]

OptionsBuilderExtensions.ValidateOnStart register StartupValidator service for option validation:

runtime/src/libraries/Microsoft.Extensions.Options/src/OptionsBuilderExtensions.cs

Line 28 in 6cc6c66

optionsBuilder.Services.AddTransient<IStartupValidator, StartupValidator>();

It does that with AddTransient, but ValidateOnStart can be called multiple times (once for each option type plus option name combination), which leads to StartupValidator would be registered multiple times. Should it use TryAddTransient instead to keep only one registration of StartupValidator?

[tarekgh]

@PetSerAl thanks for your report. Could you please submit a small sample code to demonstrate the problem you are seeing?

[PetSerAl]

using System;
using System.Linq;
using Microsoft.Extensions.DependencyInjection;
using Microsoft.Extensions.Options;

ServiceCollection sc = new();
sc.AddOptions<Option>("name1").Configure(o => o.Value = 1).Validate(o => o.Value > 0).ValidateOnStart();
sc.AddOptions<Option>("name2").Configure(o => o.Value = 2).Validate(o => o.Value > 0).ValidateOnStart();
sc.AddOptions<Option>("name3").Configure(o => o.Value = 3).Validate(o => o.Value > 0).ValidateOnStart();
sc.AddOptions<Option>("name4").Configure(o => o.Value = 4).Validate(o => o.Value > 0).ValidateOnStart();
sc.AddOptions<Option>("name5").Configure(o => o.Value = 5).Validate(o => o.Value > 0).ValidateOnStart();
sc.AddOptions<Option>("name6").Configure(o => o.Value = 6).Validate(o => o.Value > 0).ValidateOnStart();
sc.AddOptions<Option>("name7").Configure(o => o.Value = 7).Validate(o => o.Value > 0).ValidateOnStart();
sc.AddOptions<Option>("name8").Configure(o => o.Value = 8).Validate(o => o.Value > 0).ValidateOnStart();
sc.AddOptions<Option>("name9").Configure(o => o.Value = 9).Validate(o => o.Value > 0).ValidateOnStart();

foreach(var sd in sc.Where(sd => sd.ServiceType == typeof(IStartupValidator))) {
    Console.WriteLine($"{sd.ServiceType.FullName}|{sd.ImplementationType?.FullName}");
}

class Option {
    public int Value { get; set; }
}

Output:

Microsoft.Extensions.Options.IStartupValidator|Microsoft.Extensions.Options.StartupValidator
Microsoft.Extensions.Options.IStartupValidator|Microsoft.Extensions.Options.StartupValidator
Microsoft.Extensions.Options.IStartupValidator|Microsoft.Extensions.Options.StartupValidator
Microsoft.Extensions.Options.IStartupValidator|Microsoft.Extensions.Options.StartupValidator
Microsoft.Extensions.Options.IStartupValidator|Microsoft.Extensions.Options.StartupValidator
Microsoft.Extensions.Options.IStartupValidator|Microsoft.Extensions.Options.StartupValidator
Microsoft.Extensions.Options.IStartupValidator|Microsoft.Extensions.Options.StartupValidator
Microsoft.Extensions.Options.IStartupValidator|Microsoft.Extensions.Options.StartupValidator
Microsoft.Extensions.Options.IStartupValidator|Microsoft.Extensions.Options.StartupValidator

Same service type with the same implementation type registered multiple times.

[tarekgh]

@PetSerAl I think you are correct. TryAddTransient should be used instead of AddTransient.

@eerhardt @davidfowl @steveharter @ericstj do you see any risk fixing this? to summarize the issue, every time OptionsBuilder<TOptions>.ValidateOnStart() get called, it does

optionsBuilder.Services.AddTransient<IStartupValidator, StartupValidator>();

which always add transit service to the DI using the same interface type IStartupValidator and implementation type StartupValidator. Using TryAddTransient should be used instead to avoid adding the same thing multiple times.

[ericstj]

That seems correct, looks like that's what the old extensions library did: https://github.com/dotnet/extensions/blob/c9b1f395705c56438fb4f9eba351d6e0db624f12/src/ToBeMoved/Hosting.StartupInitialization/Internal/StartupInitializationBuilder.cs#L45C18-L45C33

[tarekgh]

@PetSerAl are you interested to submit a PR for the fix?

[PetSerAl]

@PetSerAl are you interested to submit a PR for the fix?

Not any time soon. I currently do not have environment prepared for this.

[tarekgh]

@PetSerAl no worries, I'll do it.