Improve integration of configuration based clients with other features (

Azure#14859)

* Improve integration of configuration based clients with other features

* Fixes

sdk/core/Microsoft.Extensions.Azure/src/Internal/IClientOptionsFactory.cs

@@ -0,0 +1,12 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+using Azure.Core;
+
+namespace Microsoft.Extensions.Azure
+{
+    internal interface IClientOptionsFactory
+    {
+        object CreateOptions(string name);
+    }
+}

sdk/extensions/Microsoft.Extensions.Azure/src/AzureClientFactoryBuilder.cs

@@ -100,7 +100,6 @@ IAzureClientBuilder<TClient, TOptions> IAzureClientFactoryBuilderWithCredential.
             _serviceCollection.TryAddSingleton(typeof(IConfigureOptions<AzureClientCredentialOptions<TClient>>), typeof(DefaultCredentialClientOptionsSetup<TClient>));
             _serviceCollection.TryAddSingleton(typeof(IOptionsMonitor<TOptions>), typeof(ClientOptionsMonitor<TClient, TOptions>));
             _serviceCollection.TryAddSingleton(typeof(ClientOptionsFactory<TClient, TOptions>), typeof(ClientOptionsFactory<TClient, TOptions>));
-            _serviceCollection.TryAddSingleton(typeof(IConfigureOptions<TOptions>), typeof(DefaultClientOptionsSetup<TOptions>));
             _serviceCollection.TryAddSingleton(typeof(IAzureClientFactory<TClient>), typeof(AzureClientFactory<TClient, TOptions>));
             _serviceCollection.TryAddSingleton(
                 typeof(TClient),

sdk/extensions/Microsoft.Extensions.Azure/src/Internal/AzureClientFactory.cs

@@ -12,15 +12,18 @@ internal class AzureClientFactory<TClient, TOptions>: IAzureClientFactory<TClien
         private readonly Dictionary<string, ClientRegistration<TClient>> _clientRegistrations;
 
         private readonly IServiceProvider _serviceProvider;
+        private readonly IOptionsMonitor<AzureClientsGlobalOptions> _globalOptions;
 
         private readonly IOptionsMonitor<AzureClientCredentialOptions<TClient>> _clientsOptions;
 
         private readonly IOptionsMonitor<TOptions> _monitor;
 
         private readonly EventSourceLogForwarder _logForwarder;
+        private FallbackAzureClientFactory<TClient> _fallbackFactory;
 
         public AzureClientFactory(
             IServiceProvider serviceProvider,
+            IOptionsMonitor<AzureClientsGlobalOptions> globalOptions,
             IOptionsMonitor<AzureClientCredentialOptions<TClient>> clientsOptions,
             IEnumerable<ClientRegistration<TClient>> clientRegistrations,
             IOptionsMonitor<TOptions> monitor,
@@ -33,6 +36,7 @@ public AzureClientFactory(
             }
 
             _serviceProvider = serviceProvider;
+            _globalOptions = globalOptions;
             _clientsOptions = clientsOptions;
             _monitor = monitor;
             _logForwarder = logForwarder;
@@ -41,9 +45,12 @@ public AzureClientFactory(
         public TClient CreateClient(string name)
         {
             _logForwarder.Start();
+
             if (!_clientRegistrations.TryGetValue(name, out ClientRegistration<TClient> registration))
             {
-                throw new InvalidOperationException($"Unable to find client registration with type '{typeof(TClient).Name}' and name '{name}'.");
+                _fallbackFactory ??= new FallbackAzureClientFactory<TClient>(_globalOptions, _serviceProvider, _logForwarder);
+
+                return _fallbackFactory.CreateClient(name);
             }
 
             return registration.GetClient(_monitor.Get(name), _clientsOptions.Get(name).CredentialFactory(_serviceProvider));

sdk/extensions/Microsoft.Extensions.Azure/src/Internal/AzureClientsGlobalOptions.cs

@@ -4,7 +4,6 @@
 using System;
 using System.Collections.Generic;
 using Azure.Core;
-using Azure.Core.Pipeline;
 using Azure.Identity;
 using Microsoft.Extensions.Configuration;
 

sdk/extensions/Microsoft.Extensions.Azure/src/Internal/ClientOptionsFactory.cs

@@ -4,23 +4,33 @@
 using System;
 using System.Collections.Generic;
 using System.Reflection;
+using Azure.Core;
 using Microsoft.Extensions.Options;
 
 namespace Microsoft.Extensions.Azure
 {
     // Slightly adjusted copy of https://github.com/aspnet/Extensions/blob/master/src/Options/Options/src/OptionsFactory.cs
-    internal class ClientOptionsFactory<TClient, TOptions> where TOptions : class
+    internal class ClientOptionsFactory<TClient, TOptions>: IClientOptionsFactory where TOptions : class
     {
         private readonly IEnumerable<IConfigureOptions<TOptions>> _setups;
         private readonly IEnumerable<IPostConfigureOptions<TOptions>> _postConfigures;
 
         private readonly IEnumerable<ClientRegistration<TClient>> _clientRegistrations;
+        private readonly IOptions<AzureClientsGlobalOptions> _defaultOptions;
+        private readonly IServiceProvider _serviceProvider;
 
-        public ClientOptionsFactory(IEnumerable<IConfigureOptions<TOptions>> setups, IEnumerable<IPostConfigureOptions<TOptions>> postConfigures, IEnumerable<ClientRegistration<TClient>> clientRegistrations)
+        public ClientOptionsFactory(
+            IEnumerable<IConfigureOptions<TOptions>> setups,
+            IEnumerable<IPostConfigureOptions<TOptions>> postConfigures,
+            IEnumerable<ClientRegistration<TClient>> clientRegistrations,
+            IOptions<AzureClientsGlobalOptions> defaultOptions,
+            IServiceProvider serviceProvider)
         {
             _setups = setups;
             _postConfigures = postConfigures;
             _clientRegistrations = clientRegistrations;
+            _defaultOptions = defaultOptions;
+            _serviceProvider = serviceProvider;
         }
 
         private TOptions CreateOptions(string name)
@@ -35,7 +45,17 @@ private TOptions CreateOptions(string name)
                 }
             }
 
-            return (TOptions)ClientFactory.CreateClientOptions(version, typeof(TOptions));
+            var options = (TOptions)ClientFactory.CreateClientOptions(version, typeof(TOptions));
+
+            if (options is ClientOptions clientOptions)
+            {
+                foreach (var globalConfigureOption in _defaultOptions.Value.ConfigureOptionDelegates)
+                {
+                    globalConfigureOption(clientOptions, _serviceProvider);
+                }
+            }
+
+            return options;
         }
 
         /// <summary>
@@ -62,5 +82,10 @@ public TOptions Create(string name)
 
             return options;
         }
+
+        object IClientOptionsFactory.CreateOptions(string name)
+        {
+            return Create(name);
+        }
     }
 }

sdk/extensions/Microsoft.Extensions.Azure/src/Internal/FallbackAzureClientFactory.cs

@@ -7,25 +7,30 @@
 using System.Reflection;
 using Azure.Core;
 using Microsoft.Extensions.Configuration;
+using Microsoft.Extensions.DependencyInjection;
 using Microsoft.Extensions.Options;
 
 namespace Microsoft.Extensions.Azure
 {
     internal class FallbackAzureClientFactory<TClient>: IAzureClientFactory<TClient>
     {
-        private readonly IOptionsMonitor<AzureClientsGlobalOptions> _globalOptions;
+        private readonly AzureClientsGlobalOptions _globalOptions;
         private readonly IServiceProvider _serviceProvider;
         private readonly EventSourceLogForwarder _logForwarder;
         private readonly Dictionary<string, FallbackClientRegistration<TClient>> _clientRegistrations;
         private readonly Type _clientOptionType;
+        private readonly IConfiguration _configurationRoot;
+        private readonly IClientOptionsFactory _optionsFactory;
 
         public FallbackAzureClientFactory(
             IOptionsMonitor<AzureClientsGlobalOptions> globalOptions,
             IServiceProvider serviceProvider,
             EventSourceLogForwarder logForwarder)
         {
-            _globalOptions = globalOptions;
             _serviceProvider = serviceProvider;
+            _globalOptions = globalOptions.CurrentValue;
+            _configurationRoot = _globalOptions.ConfigurationRootResolver?.Invoke(_serviceProvider);
+
             _logForwarder = logForwarder;
             _clientRegistrations = new Dictionary<string, FallbackClientRegistration<TClient>>();
 
@@ -43,23 +48,29 @@ public FallbackAzureClientFactory(
             {
                 throw new InvalidOperationException("Unable to detect the client option type");
             }
+
+            _optionsFactory = (IClientOptionsFactory)ActivatorUtilities.CreateInstance(serviceProvider, typeof(ClientOptionsFactory<,>).MakeGenericType(typeof(TClient), _clientOptionType));
         }
 
         public TClient CreateClient(string name)
         {
-            _logForwarder.Start();
+            if (_configurationRoot == null)
+            {
+                throw new InvalidOperationException($"Unable to find client registration with type '{typeof(TClient).Name}' and name '{name}'.");
+            }
 
-            var globalOptions = _globalOptions.CurrentValue;
+            _logForwarder.Start();
 
             FallbackClientRegistration<TClient> registration;
             lock (_clientRegistrations)
             {
                 if (!_clientRegistrations.TryGetValue(name, out registration))
                 {
-                    var section = globalOptions.ConfigurationRootResolver?.Invoke(_serviceProvider).GetSection(name);
+                    var section = _configurationRoot.GetSection(name);
+
                     if (!section.Exists())
                     {
-                        throw new InvalidOperationException($"Unable to find a configuration section with the name {name} to configure the client with or the configuration root wasn't set.");
+                        throw new InvalidOperationException($"Unable to find a configuration section with the name {name} to configure the client with.");
                     }
 
                     registration = new FallbackClientRegistration<TClient>(
@@ -71,24 +82,12 @@ public TClient CreateClient(string name)
                 }
             }
 
-
-            return registration.GetClient(
-                GetClientOptions(globalOptions, registration.Configuration),
-                ClientFactory.CreateCredential(registration.Configuration) ?? globalOptions.CredentialFactory(_serviceProvider));
+            var currentOptions = _optionsFactory.CreateOptions(name);
+            registration.Configuration.Bind(currentOptions);
+            return registration.GetClient(currentOptions,
+                ClientFactory.CreateCredential(registration.Configuration) ?? _globalOptions.CredentialFactory(_serviceProvider));
         }
 
-        private object GetClientOptions(AzureClientsGlobalOptions globalOptions, IConfiguration section)
-        {
-            var clientOptions = (ClientOptions) ClientFactory.CreateClientOptions(null, _clientOptionType);
-            foreach (var globalConfigureOptions in globalOptions.ConfigureOptionDelegates)
-            {
-                globalConfigureOptions(clientOptions, _serviceProvider);
-            }
-
-            section.Bind(clientOptions);
-
-            return clientOptions;
-        }
 
         private class FallbackClientRegistration<T>: ClientRegistration<T>
         {

sdk/extensions/Microsoft.Extensions.Azure/src/Internal/IClientOptionsFactory.cs

@@ -0,0 +1,10 @@
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License.
+
+namespace Microsoft.Extensions.Azure
+{
+    internal interface IClientOptionsFactory
+    {
+        object CreateOptions(string name);
+    }
+}

sdk/extensions/Microsoft.Extensions.Azure/tests/AzureClientFactoryTests.cs

@@ -334,6 +334,97 @@ public void CanCreateClientWithoutRegistration()
             Assert.AreEqual("ConfigurationTenantId", clientSecretCredential.TenantId);
         }
 
+        [Test]
+        public void RegistrationOverridesConfigurationBasedClient()
+        {
+            var configuration = GetConfiguration(
+                new KeyValuePair<string, string>("TestClient:connectionString", "http://localhost/"),
+                new KeyValuePair<string, string>("AnotherTestClient:connectionString", "http://betterhost/"));
+
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddAzureClients(builder => {
+                builder.AddTestClient(configuration.GetSection("AnotherTestClient")).WithName("TestClient");
+                builder.UseConfiguration(_ => configuration);
+            });
+
+            ServiceProvider provider = serviceCollection.BuildServiceProvider();
+            IAzureClientFactory<TestClient> factory = provider.GetService<IAzureClientFactory<TestClient>>();
+            TestClient client = factory.CreateClient("TestClient");
+
+            Assert.AreEqual("http://betterhost/", client.ConnectionString);
+        }
+
+        [Test]
+        public void RegistrationFallsBackToConfigurationBasedClient()
+        {
+            var configuration = GetConfiguration(
+                new KeyValuePair<string, string>("TestClient:connectionString", "http://localhost/"),
+                new KeyValuePair<string, string>("AnotherTestClient:connectionString", "http://betterhost/"));
+
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddAzureClients(builder => {
+                builder.AddTestClient(configuration.GetSection("AnotherTestClient"));
+                builder.UseConfiguration(_ => configuration);
+            });
+
+            ServiceProvider provider = serviceCollection.BuildServiceProvider();
+            IAzureClientFactory<TestClient> factory = provider.GetService<IAzureClientFactory<TestClient>>();
+            TestClient client = factory.CreateClient("TestClient");
+            TestClient defaultTestClient = provider.GetService<TestClient>();
+
+            Assert.AreEqual("http://localhost/", client.ConnectionString);
+            Assert.AreEqual("http://betterhost/", defaultTestClient.ConnectionString);
+        }
+
+        [Test]
+        public void CanSetClientOptionsInConfigurationBasedClientsViaConfigureOptions()
+        {
+            var configuration = GetConfiguration(
+                new KeyValuePair<string, string>("TestClient:connectionString", "http://localhost/"),
+                new KeyValuePair<string, string>("TestClient2:connectionString", "http://localhost2/")
+                );
+
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddAzureClients(builder => {
+                builder.UseConfiguration(_ => configuration);
+            });
+
+            serviceCollection.ConfigureAll<TestClientOptions>(options => options.Property = "client option value");
+            serviceCollection.Configure<TestClientOptions>("TestClient", options => options.IntProperty = 2);
+
+            ServiceProvider provider = serviceCollection.BuildServiceProvider();
+            IAzureClientFactory<TestClient> factory = provider.GetService<IAzureClientFactory<TestClient>>();
+            TestClient client = factory.CreateClient("TestClient");
+            TestClient client2 = factory.CreateClient("TestClient2");
+
+            Assert.AreEqual("http://localhost/", client.ConnectionString);
+            Assert.AreEqual("client option value", client.Options.Property);
+            Assert.AreEqual(2, client.Options.IntProperty);
+
+            Assert.AreEqual("http://localhost2/", client2.ConnectionString);
+            Assert.AreEqual("client option value", client2.Options.Property);
+        }
+
+        [Test]
+        public void CanSetClientOptionsInConfigurationBasedClients()
+        {
+            var configuration = GetConfiguration(
+                new KeyValuePair<string, string>("TestClient:connectionString", "http://localhost/"),
+                new KeyValuePair<string, string>("TestClient:Property", "client option value"));
+
+            var serviceCollection = new ServiceCollection();
+            serviceCollection.AddAzureClients(builder => {
+                builder.UseConfiguration(_ => configuration);
+            });
+
+            ServiceProvider provider = serviceCollection.BuildServiceProvider();
+            IAzureClientFactory<TestClient> factory = provider.GetService<IAzureClientFactory<TestClient>>();
+            TestClient client = factory.CreateClient("TestClient");
+
+            Assert.AreEqual("http://localhost/", client.ConnectionString);
+            Assert.AreEqual("client option value", client.Options.Property);
+        }
+
         [Test]
         public void CanCreateClientWithoutRegistrationUsingConnectionString()
         {