Move configuration-based client logic to Microsoft.Azure.WebJobs.Extensions.Clients

sdk/extensions/Microsoft.Azure.WebJobs.Extensions.Clients/src/AzureClientsExtensionConfigProvider.cs

@@ -2,25 +2,32 @@
 // Licensed under the MIT License.
 
 using System;
+using System.Linq;
+using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
+using Azure.Core;
 using Microsoft.Azure.WebJobs;
+using Microsoft.Azure.WebJobs.Extensions.Clients.Shared;
 using Microsoft.Azure.WebJobs.Host;
 using Microsoft.Azure.WebJobs.Host.Bindings;
 using Microsoft.Azure.WebJobs.Host.Config;
 using Microsoft.Extensions.Azure;
+using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.DependencyInjection;
 
 namespace Microsoft.Extensions.Hosting
 {
     internal class AzureClientsExtensionConfigProvider : IExtensionConfigProvider
     {
-        private readonly IServiceProvider _serviceProvider;
+        private readonly AzureComponentFactory _componentFactory;
+        private readonly IConfiguration _configuration;
         private readonly INameResolver _nameResolver;
 
-        public AzureClientsExtensionConfigProvider(IServiceProvider serviceProvider, INameResolver nameResolver)
+        public AzureClientsExtensionConfigProvider(AzureComponentFactory componentFactory, IConfiguration configuration, INameResolver nameResolver)
         {
-            _serviceProvider = serviceProvider;
+            _componentFactory = componentFactory;
+            _configuration = configuration;
             _nameResolver = nameResolver;
         }
 
@@ -32,34 +39,59 @@ public void Initialize(ExtensionConfigContext context)
 
         private IValueBinder CreateValueBinder(Type type, AzureClientAttribute attribute)
         {
-            return (IValueBinder)Activator.CreateInstance(
-                typeof(AzureClientValueProvider<>).MakeGenericType(type),
-                _serviceProvider,
-                _nameResolver.ResolveWholeString(attribute.Connection));
+            var name = _nameResolver.ResolveWholeString(attribute.Connection);
+            var section = _configuration.GetWebJobsConnectionStringSection(name);
+            if (!section.Exists())
+            {
+                throw new InvalidOperationException($"Unable to find a configuration section with the name {name} to configure the client with.");
+            }
+
+            return new AzureClientValueProvider(
+                type,
+                _componentFactory,
+                section);
         }
 
-        private class AzureClientValueProvider<TClient> : IValueBinder
+        private class AzureClientValueProvider : IValueBinder
         {
-            private readonly IServiceProvider _serviceProvider;
-            private readonly string _connection;
+            private readonly Type _clientType;
+            private readonly AzureComponentFactory _componentFactory;
+            private readonly IConfigurationSection _configuration;
 
-            public AzureClientValueProvider(IServiceProvider serviceProvider, string connection)
+            public AzureClientValueProvider(Type clientType, AzureComponentFactory componentFactory, IConfigurationSection configuration)
             {
-                _serviceProvider = serviceProvider;
-                _connection = connection;
+                _clientType = clientType;
+                _componentFactory = componentFactory;
+                _configuration = configuration;
             }
 
             public Task<object> GetValueAsync()
             {
-                return Task.FromResult(
-                    (object)_serviceProvider
-                        .GetRequiredService<IAzureClientFactory<TClient>>()
-                        .CreateClient(_connection));
+                Type clientOptionType = null;
+                foreach (var constructor in _clientType.GetConstructors(BindingFlags.Public | BindingFlags.Instance))
+                {
+                    var lastParameter = constructor.GetParameters().LastOrDefault();
+                    if (lastParameter != null && typeof(ClientOptions).IsAssignableFrom(lastParameter.ParameterType))
+                    {
+                        clientOptionType = lastParameter.ParameterType;
+                        break;
+                    }
+                }
+
+                if (clientOptionType == null)
+                {
+                    throw new InvalidOperationException("Unable to detect the client option type");
+                }
+
+                var credential = _componentFactory.CreateCredential(_configuration);
+                var options = _componentFactory.CreateClientOptions(clientOptionType, null, _configuration);
+
+                return Task.FromResult(_componentFactory.CreateClient(_clientType, _configuration, credential, options));
             }
 
-            public string ToInvokeString() => $"{typeof(TClient).Name} Connection: {_connection}";
+            public string ToInvokeString() => $"{_clientType.Name} Connection: {_configuration.Path}";
 
-            public Type Type => typeof(TClient);
+            public Type Type => _clientType;
 
             public Task SetValueAsync(object value, CancellationToken cancellationToken)
             {

sdk/extensions/Microsoft.Azure.WebJobs.Extensions.Clients/src/AzureClientsWebJobsBuilderExtensions.cs

@@ -29,8 +29,7 @@ public static IWebJobsBuilder AddAzureClients(this IWebJobsBuilder builder)
                 throw new ArgumentNullException(nameof(builder));
             }
 
-            builder.Services.AddAzureClients(builder =>
-                builder.UseConfiguration(provider => new WebJobsConfiguration(provider.GetRequiredService<IConfiguration>())));
+            builder.Services.AddAzureClientsCore();
             builder.AddExtension<AzureClientsExtensionConfigProvider>();
 
             return builder;

sdk/storage/Azure.Storage.Webjobs.Extensions.Common/src/Shared/IConfigurationExtensions.cs → sdk/extensions/Microsoft.Azure.WebJobs.Extensions.Clients/src/Shared/WebJobsConfigurationExtensions.cs

@@ -3,10 +3,12 @@
 
 using Microsoft.Extensions.Configuration;
 
-namespace Microsoft.Azure.WebJobs.Extensions.Storage.Common
+namespace Microsoft.Azure.WebJobs.Extensions.Clients.Shared
 {
-    internal static class IConfigurationExtensions
+    internal static class WebJobsConfigurationExtensions
     {
+        private const string WebJobsConfigurationSectionName = "AzureWebJobs";
+
         public static IConfigurationSection GetWebJobsConnectionStringSection(this IConfiguration configuration, string connectionStringName)
         {
             // first try prefixing
@@ -24,7 +26,7 @@ public static IConfigurationSection GetWebJobsConnectionStringSection(this IConf
 
         public static string GetPrefixedConnectionStringName(string connectionStringName)
         {
-            return Constants.WebJobsConfigurationSectionName + connectionStringName;
+            return WebJobsConfigurationSectionName + connectionStringName;
         }
 
         /// <summary>

sdk/extensions/Microsoft.Azure.WebJobs.Extensions.Clients/tests/WebJobsConfigurationTests.cs

@@ -2,6 +2,7 @@
 // Licensed under the MIT License.
 
 using System.Collections.Generic;
+using Microsoft.Azure.WebJobs.Extensions.Clients.Shared;
 using Microsoft.Extensions.Configuration;
 using Microsoft.Extensions.Hosting;
 using NUnit.Framework;
@@ -20,9 +21,8 @@ public void KeysAreMappedForSingleValue(string actualKeyName)
                 {
                     new KeyValuePair<string, string>(actualKeyName, "value")
                 });
-            var configuration = new WebJobsConfiguration(builder.Build());
-
-            Assert.AreEqual(configuration.GetSection("Key").Value, "value");
+            var configuration = builder.Build();
+            Assert.AreEqual(configuration.GetWebJobsConnectionStringSection("Key").Value, "value");
         }
 
         [TestCase("Key")]
@@ -36,10 +36,10 @@ public void KeysAreMappedForSections(string actualKeyName)
                     new KeyValuePair<string, string>(actualKeyName + ":Value1", "value1"),
                     new KeyValuePair<string, string>(actualKeyName + ":Value2", "value2")
                 });
-            var configuration = new WebJobsConfiguration(builder.Build());
+            var configuration = builder.Build();
 
-            Assert.AreEqual(configuration.GetSection("Key")["Value1"], "value1");
-            Assert.AreEqual(configuration.GetSection("Key")["Value2"], "value2");
+            Assert.AreEqual(configuration.GetWebJobsConnectionStringSection("Key")["Value1"], "value1");
+            Assert.AreEqual(configuration.GetWebJobsConnectionStringSection("Key")["Value2"], "value2");
         }
     }
 }

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

@@ -127,18 +127,5 @@ public AzureClientFactoryBuilder UseCredential(Func<IServiceProvider, TokenCrede
             _serviceCollection.Configure<AzureClientsGlobalOptions>(options => options.CredentialFactory = tokenCredentialFactory);
             return this;
         }
-
-        /// <summary>
-        /// Sets the configuration instance that is used to resolve clients that were not explicitly registered.
-        /// </summary>
-        /// <param name="configurationProvider">The delegate that returns a configuration instance that's used to resolve client configuration from.</param>
-        /// <returns>This instance.</returns>
-        public AzureClientFactoryBuilder UseConfiguration(Func<IServiceProvider, IConfiguration> configurationProvider)
-        {
-            _serviceCollection.Configure<AzureClientsGlobalOptions>(options => options.ConfigurationRootResolver = configurationProvider);
-
-            return this;
-        }
-
     }
 }

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

@@ -22,20 +22,19 @@ public static class AzureClientServiceCollectionExtensions
         /// <param name="configureClients">An <see cref="AzureClientFactoryBuilder"/> that can be used to configure the client.</param>
         public static void AddAzureClients(this IServiceCollection collection, Action<AzureClientFactoryBuilder> configureClients)
         {
-            collection.AddAzureClientCore();
+            collection.AddAzureClientsCore();
             configureClients(new AzureClientFactoryBuilder(collection));
         }
 
         /// <summary>
         /// Adds the minimum essential Azure SDK interop services like <see cref="AzureEventSourceLogForwarder"/> and <see cref="AzureComponentFactory"/> to the specified <see cref="IServiceCollection"/> without registering any client types.
         /// </summary>
         /// <param name="collection">The <see cref="IServiceCollection"/>.</param>
-        public static void AddAzureClientCore(this IServiceCollection collection)
+        public static void AddAzureClientsCore(this IServiceCollection collection)
         {
             collection.AddOptions();
             collection.TryAddSingleton<AzureEventSourceLogForwarder>();
             collection.TryAddSingleton<ILoggerFactory, NullLoggerFactory>();
-            collection.TryAddSingleton(typeof(IAzureClientFactory<>), typeof(FallbackAzureClientFactory<>));
             collection.TryAddSingleton<AzureComponentFactory, AzureComponentFactoryImpl>();
         }
     }

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

@@ -19,17 +19,14 @@ internal class AzureClientFactory<TClient, TOptions>: IAzureClientFactory<TClien
         private readonly IOptionsMonitor<TOptions> _monitor;
 
         private readonly AzureEventSourceLogForwarder _logForwarder;
-        private readonly AzureComponentFactory _componentFactory;
-        private FallbackAzureClientFactory<TClient> _fallbackFactory;
 
         public AzureClientFactory(
             IServiceProvider serviceProvider,
             IOptionsMonitor<AzureClientsGlobalOptions> globalOptions,
             IOptionsMonitor<AzureClientCredentialOptions<TClient>> clientsOptions,
             IEnumerable<ClientRegistration<TClient>> clientRegistrations,
             IOptionsMonitor<TOptions> monitor,
-            AzureEventSourceLogForwarder logForwarder,
-            AzureComponentFactory componentFactory)
+            AzureEventSourceLogForwarder logForwarder)
         {
             _clientRegistrations = new Dictionary<string, ClientRegistration<TClient>>();
             foreach (var registration in clientRegistrations)
@@ -42,7 +39,6 @@ public AzureClientFactory(
             _clientsOptions = clientsOptions;
             _monitor = monitor;
             _logForwarder = logForwarder;
-            _componentFactory = componentFactory;
         }
 
         public TClient CreateClient(string name)
@@ -51,12 +47,7 @@ public TClient CreateClient(string name)
 
             if (!_clientRegistrations.TryGetValue(name, out ClientRegistration<TClient> registration))
             {
-                _fallbackFactory ??= new FallbackAzureClientFactory<TClient>(
-                    _globalOptions,
-                    _serviceProvider,
-                    _componentFactory,
-                    _logForwarder);
-                return _fallbackFactory.CreateClient(name);
+                throw new InvalidOperationException($"Unable to find client registration with type '{typeof(TClient).Name}' and name '{name}'.");
             }
 
             return registration.GetClient(_monitor.Get(name), _clientsOptions.Get(name).CredentialFactory(_serviceProvider));

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

@@ -25,5 +25,16 @@ public abstract class AzureComponentFactory
         /// <param name="configuration">The <see cref="IConfiguration"/> instance to apply to options.</param>
         /// <returns>A new instance of <paramref name="optionsType"/>.</returns>
         public abstract object CreateClientOptions(Type optionsType, object serviceVersion, IConfiguration configuration);
+
+        /// <summary>
+        /// Creates a new client instance using the provided configuration to map constructor parameters from.
+        /// Optionally takes a set of client option and credential to use when constructing a client.
+        /// </summary>
+        /// <param name="clientType"></param>
+        /// <param name="configuration">The <see cref="IConfiguration"/> instance to map constructor parameters from.</param>
+        /// <param name="credential">The <see cref="TokenCredential"/> object to use if required by constructor, if null no .</param>
+        /// <param name="clientOptions">The client </param>
+        /// <returns></returns>
+        public abstract object CreateClient(Type clientType, IConfiguration configuration, TokenCredential credential, object clientOptions);
     }
 }

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

@@ -27,6 +27,7 @@ public override TokenCredential CreateCredential(IConfiguration configuration)
 
         public override object CreateClientOptions(Type optionsType, object serviceVersion, IConfiguration configuration)
         {
+            if (optionsType == null) throw new ArgumentNullException(nameof(optionsType));
             var options = ClientFactory.CreateClientOptions(serviceVersion, optionsType);
 
             if (options is ClientOptions clientOptions)
@@ -39,5 +40,14 @@ public override object CreateClientOptions(Type optionsType, object serviceVersi
             configuration?.Bind(options);
             return options;
         }
+
+        public override object CreateClient(Type clientType, IConfiguration configuration, TokenCredential credential, object clientOptions)
+        {
+            if (clientType == null) throw new ArgumentNullException(nameof(clientType));
+            if (configuration == null) throw new ArgumentNullException(nameof(configuration));
+            if (clientOptions == null) throw new ArgumentNullException(nameof(clientOptions));
+
+            return ClientFactory.CreateClient(clientType, clientOptions.GetType(), clientOptions, configuration, credential);
+        }
     }
 }