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

Improve integration of configuration based clients with other features #14859

Merged
3 commits merged into from
Sep 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -33,6 +36,7 @@ public AzureClientFactory(
}

_serviceProvider = serviceProvider;
_globalOptions = globalOptions;
_clientsOptions = clientsOptions;
_monitor = monitor;
_logForwarder = logForwarder;
Expand All @@ -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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@
using System;
using System.Collections.Generic;
using Azure.Core;
using Azure.Core.Pipeline;
using Azure.Identity;
using Microsoft.Extensions.Configuration;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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>
Expand All @@ -62,5 +82,10 @@ public TOptions Create(string name)

return options;
}

object IClientOptionsFactory.CreateOptions(string name)
{
return Create(name);
}
}
}

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -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>>();

Expand All @@ -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>(
Expand All @@ -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>
{
Expand Down
Original file line number Diff line number Diff line change
@@ -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);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down