AddAzureAppConfiguration - Use TryAddSingleton instead of AddSingleton

[MarienMonnier]

Hello,

We are using AAC with 2 endpoints:

• one storing the configuration
• one storing the feature flags, using Microsoft.FeatureManagement

We are calling AddAzureAppConfiguration for both (some application only use the one for configuration, others the one for feature flags).
This ends up in having two instances of the singleton IConfigurationRefresherProvider in the dependency injection provider. This causes issue on our integration tests where we try to replace the first instance.

Would you accept a change to replace the AddSingleton with TryAddSingleton here?
https://github.com/Azure/AppConfiguration-DotnetProvider/blob/main/src/Microsoft.Extensions.Configuration.AzureAppConfiguration/AzureAppConfigurationExtensions.cs#L99C1-L107C14

If yes, I can propose a Pull Request.

Thanks,

[amerjusupovic]

Hello @MarienMonnier, I don't see a problem with it right now other than figuring out the implementation. The Refreshers object inside AzureAppConfigurationRefresherProvider is only instantiated in the constructor. Because we call AddSingleton, the constructor is called again, updating Refreshers and checking it's valid. If you feel you can address that in your PR (keeping the functionality consistent with how it is now), we can definitely review it. Thanks!

[amerjusupovic]

Actually @MarienMonnier I don't think you should need to call AddAzureAppConfiguration(this IServiceCollection services) twice. This can just be called once after all configuration providers are added using the IConfigurationBuilder.AddAzureAppConfiguration method twice, and the AzureAppConfigurationRefresherProvider can discover all of the provider instances at that point.

Sorry for the confusion, let me know if you have more questions or need help restructuring your application.

[jimmyca15]

@amerjusupovic while I agree it should only need to be called once, and that calling it just once should fix this particular issue, we should still aim for it to be idempotent. Can we address this?

[amerjusupovic]

we should still aim for it to be idempotent.

That makes sense, I can work on addressing this in a PR.

[amerjusupovic]

@MarienMonnier I've merged the PR to use TryAddSingleton, and we'll plan to have this out in the next stable release. Thanks!