Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

AddOption/Service.Configure creating duplicated entries #38569

Closed
jhonathanalmeida opened this issue Nov 22, 2021 · 6 comments
Closed

AddOption/Service.Configure creating duplicated entries #38569

jhonathanalmeida opened this issue Nov 22, 2021 · 6 comments
Assignees
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-hosting ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved

Comments

@jhonathanalmeida
Copy link

jhonathanalmeida commented Nov 22, 2021

Describe the bug

When injecting settings on WebApplication Builder, 2 instances of the same injections are made:
builder.Services.Configure<MySettings>(builder.Configuration.GetSection("MySettings"));
OR
builder.Services.AddOptions<MySettings>().Bind(builder.Configuration.GetSection("MySettings"));

This only happens when using the WebApplication Builder, but works fine when using the old Generic Host through Program.Main + Startup file.

P.S. Found this behavior when migrating a new project from .Net5 to .Net6.

To Reproduce

Issue can be reproduced by using the following solution: https://github.com/jhonathanalmeida/DotNet6_AddOption_Test

There are two projects, one testing with GRPC and other with REST/Web.

In both projects you can see that there is only one injection of the settings, such as:
builder.Services.AddOptions<MySettings>().Bind(builder.Configuration.GetSection("MySettings"));

And then, once the application is running, you can call either the "WeatherForecastController" or "GreeterService", and during the constructor call you will see that it will log an error as it has more then one item in the list, and then throw the "InvalidOperationException" with the message " Sequence contains more than one element".

Exceptions (if any)

The exception occurs when calling the IEnumerable<IOptions<MySettings>> mySettings.Single().Value.

System.InvalidOperationException
  HResult=0x80131509
  Message=Sequence contains more than one element
  Source=System.Linq
  StackTrace:
   at System.Linq.ThrowHelper.ThrowMoreThanOneElementException()
   at System.Linq.Enumerable.TryGetSingle[TSource](IEnumerable`1 source, Boolean& found)
   at System.Linq.Enumerable.Single[TSource](IEnumerable`1 source)
   at WebAPICore.Controllers.WeatherForecastController..ctor(ILogger`1 logger, IEnumerable`1 mySettings) in C:\Users\jhonathan.almeida\source\repos\DotNet6_AddOption_Test\WebAPICore\Controllers\WeatherForecastController.cs:line 30
   at Microsoft.AspNetCore.Mvc.Controllers.ControllerActivatorProvider.<>c__DisplayClass7_0.<CreateActivator>b__0(ControllerContext controllerContext)
   at Microsoft.AspNetCore.Mvc.Controllers.ControllerFactoryProvider.<>c__DisplayClass6_0.<CreateControllerFactory>g__CreateController|0(ControllerContext controllerContext)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.Next(State& next, Scope& scope, Object& state, Boolean& isCompleted)
   at Microsoft.AspNetCore.Mvc.Infrastructure.ControllerActionInvoker.InvokeInnerFilterAsync()

Further technical details

  • The IDE (VS / VS Code/ VS4Mac) you're running on, and its version: VS 2022
  • Include the output of dotnet --info:

.NET SDK (reflecting any global.json):
Version: 6.0.100
Commit: 9e8b04bbff

Runtime Environment:
OS Name: Windows
OS Version: 10.0.19043
OS Platform: Windows
RID: win10-x64
Base Path: C:\Program Files\dotnet\sdk\6.0.100\

Host (useful for support):
Version: 6.0.0
Commit: 4822e3c3aa

.NET SDKs installed:
5.0.402 [C:\Program Files\dotnet\sdk]
6.0.100 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
Microsoft.AspNetCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 3.1.20 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.11 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 5.0.12 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.AspNetCore.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
Microsoft.NETCore.App 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 3.1.20 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.11 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 5.0.12 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.NETCore.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
Microsoft.WindowsDesktop.App 3.1.20 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.11 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 5.0.12 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
Microsoft.WindowsDesktop.App 6.0.0 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

@ghost
Copy link

ghost commented Nov 22, 2021

Thanks for contacting us.
We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. Because it's not immediately obvious that this is a bug in our framework, we would like to keep this around to collect more feedback, which can later help us determine the impact of it. We will re-evaluate this issue, during our next planning meeting(s).
If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues.
To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

@halter73
Copy link
Member

This is the result of how WebApplicationBuilder currently shares service descriptors with the underlying HostBuilder. I recently opened dotnet/runtime#61635 to improve this integration in .NET 7. I think we'll want to share the IServiceCollection more directly going forward.

AddOptions wouldn't normally ever add service descriptors more than once, but WebApplicationBuilder effectively calls AddOptions on two different IServiceCollections and merges these collections together.

I'm a little surprised you found this @jhonathanalmeida. Why are the apps trying to resolve IEnumerable<IOptions<MySettings>> to begin with instead of just Options<MySettings>? We could consider patching to remove the duplicate service descriptors. Is there code in the wild that relies on there only being exactly one IOptions<T> service?

Stacks where AddOptions is called
Microsoft.Extensions.Options.dll!Microsoft.Extensions.DependencyInjection.OptionsServiceCollectionExtensions.AddOptions(Microsoft.Extensions.DependencyInjection.IServiceCollection services)	Unknown
Microsoft.Extensions.Logging.dll!Microsoft.Extensions.DependencyInjection.LoggingServiceCollectionExtensions.AddLogging(Microsoft.Extensions.DependencyInjection.IServiceCollection services, System.Action<Microsoft.Extensions.Logging.ILoggingBuilder> configure)	Unknown
Microsoft.Extensions.Hosting.dll!Microsoft.Extensions.Hosting.HostingHostBuilderExtensions.ConfigureLogging.AnonymousMethod__0(Microsoft.Extensions.Hosting.HostBuilderContext context, Microsoft.Extensions.DependencyInjection.IServiceCollection collection)	Unknown
Microsoft.AspNetCore.dll!Microsoft.AspNetCore.Hosting.BootstrapHostBuilder.RunDefaultCallbacks(Microsoft.Extensions.Configuration.ConfigurationManager configuration, Microsoft.Extensions.Hosting.HostBuilder innerBuilder)	Unknown
Microsoft.AspNetCore.dll!Microsoft.AspNetCore.Builder.WebApplicationBuilder.WebApplicationBuilder(Microsoft.AspNetCore.Builder.WebApplicationOptions options, System.Action<Microsoft.Extensions.Hosting.IHostBuilder> configureDefaults)	Unknown
Microsoft.AspNetCore.dll!Microsoft.AspNetCore.Builder.WebApplication.CreateBuilder(string[] args)	Unknown
WebAPICore.dll!Program.<Main>$(string[] args) Line 3	C#
Microsoft.Extensions.Options.dll!Microsoft.Extensions.DependencyInjection.OptionsServiceCollectionExtensions.AddOptions(Microsoft.Extensions.DependencyInjection.IServiceCollection services)	Unknown
Microsoft.Extensions.Hosting.dll!Microsoft.Extensions.Hosting.HostBuilder.CreateServiceProvider()	Unknown
Microsoft.Extensions.Hosting.dll!Microsoft.Extensions.Hosting.HostBuilder.Build()	Unknown
Microsoft.AspNetCore.dll!Microsoft.AspNetCore.Builder.WebApplicationBuilder.Build()	Unknown
WebAPICore.dll!Program.<Main>$(string[] args) Line 21	C#

@jhonathanalmeida
Copy link
Author

jhonathanalmeida commented Nov 23, 2021

Hey @halter73, thanks for sharing that info!

Our real life scenario is actually a bit more complex.

We are developing a shared library for our internal developers, and a few of the implementations we have for instance, for Azure KeyVault, or Azure ServiceBus, we rely on injecting the configuration using AddOptions.

The thing is, one of the teams needed to actually access two different instances of the service bus, in the same application, so we tried to use named injections, which .Net 5 DI does not allow right? So what we did is, we created Identifiable Configurations. This is done by having a IIdentifiableConfiguration interface, like:

public interface IIdentifiableConfiguration
{
    string ConfigurationName { get; set; }
}

And then each injection will have this value set to it's respective "ConfigurationName ", so each service/instance can retrieve their own version of the configuration.

To exemplify, let's say we have a DBSettings class, and I need multiple injections of if, because my App need to connect to two different DBs.

So DBSettings implements this interface, and we get:

public class DBSettings : IIdentifiableConfiguration
{
    public string ConfigurationName { get; set; } = string.Empty;
    public string ConnStr { get; set; } = string.Empty;
}

And then, we would have something like this in the AppSettings:

{
"DBSettings": {
    "DB_A": {
      "ConfigurationName": "DBAKey",
      "ConnStr": "DB A Conn Str"
    },
    "DB_B": {
      "ConfigurationName": "DBBKey",
      "ConnStr": "DB B Conn Str"
    }
  } 

And then we would have these injections:

builder.Services.AddOptions<DBSettings>()
    .Bind(builder.Configuration.GetSection("DBSettings:DB_A"));
builder.Services.AddOptions<DBSettings>()
    .Bind(builder.Configuration.GetSection("DBSettings:DB_B"));

And finally in the Service we would have:

public class GreeterServiceA
{
    private const string DBConnKey = "DBAKey";

    private readonly DBSettings _dbSettings;

    public GreeterServiceA(IEnumerable<IOptions<DBSettings>> allDBSettings)
    {
        _dbSettings= allDBSettings.Select(sc => sc.Value)
            .Single(sc => sc.ConfigurationName == DBConnKey );
    }

    .
    .
    .
}

This way, each service can identify it's own instance of the configuration.

I know it is a bit messy, I'll create a realish example, and push it to the Repo above.

@jhonathanalmeida
Copy link
Author

jhonathanalmeida commented Nov 23, 2021

Ok, just pushed another example containing a somewhat similar approach to what we are doing. You can find the code in this project (under the solution/repo above).

However I'd like to highlight two points.
1- Named option injections exists (I really missed it out on our initial research), and I was able to make it work fine (commented out lines in program.cs + _namedOptionsAcessor in GreeterServiceA).
2- When trying to inject two configurations based on the same object, only the last instance prevails, and everything else is discarded. That can be tested by calling the GreeterServiceA, it will throw an exception stating that there is no matching element for "ServiceAKey".

Just for reference, this pattern of option injection we were using was working fine on .Net 5.

Also, just to give you some more info, in our real implementation, there are also two instances of the same object injected, for example a DBClient, so in order to identify them, each one is injected with a identification as well, and this is done pretty similarly to what we did with the Configurations, by implementing a IIdentifiableObject interface, which has a Key, which is then injected in the instance by a factory patter.

We just found out that named option Injections exists, but I don't think named injections exists, do they? So I can inject multiple instances of the same class/service?

Thanks once again for all the help/information.

Best regards

Jhonny

@halter73
Copy link
Member

However I'd like to highlight two points.
1- Named option injections exists (I really missed it out on our initial research), and I was able to make it work fine (commented out lines in program.cs + _namedOptionsAcessor in GreeterServiceA).

Glad to hear this! Named options do seem like the right approach here.

2- When trying to inject two configurations based on the same object, only the last instance prevails, and everything else is discarded. That can be tested by calling the GreeterServiceA, it will throw an exception stating that there is no matching element for "ServiceAKey".

This is by design. The same thing should happen in .NET 5.

We just found out that named option Injections exists, but I don't think named injections exists, do they? So I can inject multiple instances of the same class/service?

Correct. Named services aren't a concept that's built in to Microsoft.Extensions.DependencyInjection. We recommend using the factory pattern if you want named services similar to what we do with IOptionsFactory<TOptions>.

Just for reference, this pattern of option injection we were using was working fine on .Net 5.

Do you have an example project demonstrating the pattern you're trying to use working in .NET 5? I think that would better help me understand the scenario.

I don't think calling AddOptions multiple times should have ever added more than one IOptions<TOptions> service in .NET 5 meaning IEnumerable<IOptions<DBSettings>> allDBSettings should have never had more than one element unless the app was adding an IOptions<DBSettings> service using something other than AddOptions or it was merging IServiceCollections in a similar way to WebApplicationBuilder.

@rafikiassumani-msft rafikiassumani-msft added the ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. label Jan 6, 2022
@ghost ghost added the Status: Resolved label Jan 6, 2022
@ghost
Copy link

ghost commented Jan 7, 2022

This issue has been resolved and has not had any activity for 1 day. It will be closed for housekeeping purposes.

See our Issue Management Policies for more information.

@ghost ghost closed this as completed Jan 7, 2022
@ghost ghost locked as resolved and limited conversation to collaborators Feb 6, 2022
@amcasey amcasey added area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc and removed area-runtime labels Jun 2, 2023
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-minimal Includes minimal APIs, endpoint filters, parameter binding, request delegate generator etc feature-minimal-hosting ✔️ Resolution: Answered Resolved because the question asked by the original author has been answered. Status: Resolved
Projects
None yet
Development

No branches or pull requests

5 participants