Skip to content

Commit

Permalink
Remove calls to IServiceCollection.BuildServiceProvider() where possi…
Browse files Browse the repository at this point in the history 
…ble (#1235)
  • Loading branch information
@bart-vmware
bart-vmware authored Dec 21, 2023
1 parent 02eea6a commit e934355
Show file tree
Hide file tree
Showing 8 changed files with 218 additions and 185 deletions.
2 changes: 1 addition & 1 deletion src/Common/test/Common.Security.Test/CertificateRotationServiceTest.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ public async Task ServiceLoadsCertificate()
services.AddSingleton<IConfigureOptions<CertificateOptions>, ConfigureCertificateOptions>();
services.AddSingleton<ICertificateRotationService, CertificateRotationService>();

using ServiceProvider serviceProvider = services.BuildServiceProvider();
using ServiceProvider serviceProvider = services.BuildServiceProvider(true);
var service = serviceProvider.GetRequiredService<ICertificateRotationService>();

service.Start();
Expand Down
14 changes: 7 additions & 7 deletions src/Configuration/src/ConfigServer/ConfigServerDiscoveryService.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,9 @@ public ConfigServerDiscoveryService(IConfiguration configuration, ConfigServerCl
// Create a discovery client to be used (hopefully only) during startup
private void SetupDiscoveryClient()
{
var services = new ServiceCollection();
services.AddSingleton(LoggerFactory);
services.TryAdd(ServiceDescriptor.Singleton(typeof(ILogger<>), typeof(Logger<>)));
var tempServices = new ServiceCollection();
tempServices.AddSingleton(LoggerFactory);
tempServices.TryAdd(ServiceDescriptor.Singleton(typeof(ILogger<>), typeof(Logger<>)));

// force settings to make sure we don't register the app here
IConfigurationBuilder builder = new ConfigurationBuilder().AddConfiguration(_configuration).AddInMemoryCollection(new Dictionary<string, string>
Expand All @@ -52,11 +52,11 @@ private void SetupDiscoveryClient()
{ "Consul:Discovery:Register", "false" }
});

services.AddSingleton<IConfiguration>(builder.Build());
services.AddDiscoveryClient(_configuration);
tempServices.AddSingleton<IConfiguration>(builder.Build());
tempServices.AddDiscoveryClient(_configuration);

using ServiceProvider startupServiceProvider = services.BuildServiceProvider();
DiscoveryClient = startupServiceProvider.GetRequiredService<IDiscoveryClient>();
using ServiceProvider tempServiceProvider = tempServices.BuildServiceProvider();
DiscoveryClient = tempServiceProvider.GetRequiredService<IDiscoveryClient>();
_logger.LogDebug("Found Discovery Client of type {DiscoveryClientType}", DiscoveryClient.GetType());
}

Expand Down
4 changes: 2 additions & 2 deletions src/Discovery/src/Client/DiscoveryHostBuilderExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static class DiscoveryHostBuilderExtensions
/// </exception>
public static IHostBuilder AddDiscoveryClient(this IHostBuilder hostBuilder)
{
return hostBuilder.ConfigureServices((context, collection) => collection.AddDiscoveryClient(context.Configuration));
return hostBuilder.ConfigureServices((context, services) => services.AddDiscoveryClient(context.Configuration));
}

/// <summary>
Expand All @@ -55,6 +55,6 @@ public static IHostBuilder AddDiscoveryClient(this IHostBuilder hostBuilder)
/// </exception>
public static IHostBuilder AddServiceDiscovery(this IHostBuilder hostBuilder, Action<DiscoveryClientBuilder> optionsAction)
{
return hostBuilder.ConfigureServices(collection => collection.AddServiceDiscovery(optionsAction));
return hostBuilder.ConfigureServices((context, services) => services.AddServiceDiscovery(context.Configuration, optionsAction));
}
}
92 changes: 59 additions & 33 deletions src/Discovery/src/Client/DiscoveryServiceCollectionExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,27 @@ public static class DiscoveryServiceCollectionExtensions
/// <param name="configuration">
/// Application configuration.
/// </param>
public static IServiceCollection AddDiscoveryClient(this IServiceCollection services, IConfiguration configuration = null)
public static IServiceCollection AddDiscoveryClient(this IServiceCollection services, IConfiguration configuration)
{
return services.AddDiscoveryClient(configuration, null);
return AddDiscoveryClient(services, configuration, null, null);
}

/// <summary>
/// Adds service discovery to your application. Uses reflection to determine which clients are available and configured. If no clients are available or
/// configured, a <see cref="NoOpDiscoveryClient" /> will be configured.
/// </summary>
/// <param name="services">
/// <see cref="IServiceCollection" /> to configure.
/// </param>
/// <param name="configuration">
/// Application configuration.
/// </param>
/// <param name="serviceName">
/// Specify the name of a service binding to use.
/// </param>
public static IServiceCollection AddDiscoveryClient(this IServiceCollection services, IConfiguration configuration, string serviceName)
{
return AddDiscoveryClient(services, configuration, serviceName, null);
}

/// <summary>
Expand All @@ -48,9 +66,9 @@ public static IServiceCollection AddDiscoveryClient(this IServiceCollection serv
/// <param name="lifecycle">
/// Add custom code for app shutdown events.
/// </param>
public static IServiceCollection AddDiscoveryClient(this IServiceCollection services, IConfiguration configuration, IDiscoveryLifecycle lifecycle = null)
public static IServiceCollection AddDiscoveryClient(this IServiceCollection services, IConfiguration configuration, IDiscoveryLifecycle lifecycle)
{
return services.AddDiscoveryClient(configuration, null, lifecycle);
return AddDiscoveryClient(services, configuration, null, lifecycle);
}

/// <summary>
Expand All @@ -69,12 +87,13 @@ public static IServiceCollection AddDiscoveryClient(this IServiceCollection serv
/// <param name="lifecycle">
/// Add custom code for app shutdown events.
/// </param>
public static IServiceCollection AddDiscoveryClient(this IServiceCollection services, IConfiguration configuration, string serviceName = null,
IDiscoveryLifecycle lifecycle = null)
public static IServiceCollection AddDiscoveryClient(this IServiceCollection services, IConfiguration configuration, string serviceName,
IDiscoveryLifecycle lifecycle)
{
Action<DiscoveryClientBuilder> builderAction = null;
ArgumentGuard.NotNull(services);
ArgumentGuard.NotNull(configuration);

configuration ??= services.BuildServiceProvider().GetRequiredService<IConfiguration>();
Action<DiscoveryClientBuilder> builderAction = null;

IServiceInfo info = string.IsNullOrEmpty(serviceName)
? GetSingletonDiscoveryServiceInfo(configuration)
Expand All @@ -97,13 +116,13 @@ public static IServiceCollection AddDiscoveryClient(this IServiceCollection serv
else if (implementations.Count > 1)
{
// if none configured, that's ok because AddServiceDiscovery has a plan
IEnumerable<IDiscoveryClientExtension> configured = implementations.Where(client => client.IsConfigured(configuration, info));
IDiscoveryClientExtension[] configured = implementations.Where(client => client.IsConfigured(configuration, info)).ToArray();

if (configured.Count() == 1)
if (configured.Length == 1)
{
builderAction = builder => builder.Extensions.Add(configured.Single());
}
else if (configured.Count() > 1)
else if (configured.Length > 1)
{
throw new AmbiguousMatchException(
"Multiple IDiscoveryClient implementations have been added and configured! This is not supported, please only configure a single client type.");
Expand All @@ -115,16 +134,19 @@ public static IServiceCollection AddDiscoveryClient(this IServiceCollection serv
services.AddSingleton(lifecycle);
}

return services.AddServiceDiscovery(builderAction);
return AddServiceDiscovery(services, configuration, builderAction);
}

/// <summary>
/// Adds service discovery to your application. If <paramref name="builderAction" /> is not provided, a <see cref="NoOpDiscoveryClient" /> will be
/// configured.
/// </summary>
/// <param name="serviceCollection">
/// <param name="services">
/// <see cref="IServiceCollection" /> to configure.
/// </param>
/// <param name="configuration">
/// Application configuration.
/// </param>
/// <param name="builderAction">
/// Provided by the desired <see cref="IDiscoveryClient" /> implementation.
/// </param>
Expand All @@ -137,21 +159,23 @@ public static IServiceCollection AddDiscoveryClient(this IServiceCollection serv
/// <exception cref="ConnectorException">
/// Thrown if no service info with expected name or type are found or when multiple service infos are found and a single was expected.
/// </exception>
public static IServiceCollection AddServiceDiscovery(this IServiceCollection serviceCollection, Action<DiscoveryClientBuilder> builderAction = null)
public static IServiceCollection AddServiceDiscovery(this IServiceCollection services, IConfiguration configuration,
Action<DiscoveryClientBuilder> builderAction = null)
{
ArgumentGuard.NotNull(serviceCollection);
ArgumentGuard.NotNull(services);
ArgumentGuard.NotNull(configuration);

builderAction ??= builder => builder.Extensions.Add(new NoOpDiscoveryClientExtension());

serviceCollection.RegisterDefaultApplicationInstanceInfo();
ApplyDiscoveryOptions(serviceCollection, builderAction);
services.RegisterDefaultApplicationInstanceInfo();
ApplyDiscoveryOptions(services, configuration, builderAction);

serviceCollection.TryAddTransient<DiscoveryHttpMessageHandler>();
serviceCollection.AddSingleton<IServiceInstanceProvider>(p => p.GetService<IDiscoveryClient>());
serviceCollection.AddHttpClient("DiscoveryRandom").AddRandomLoadBalancer();
serviceCollection.AddHttpClient("DiscoveryRoundRobin").AddRoundRobinLoadBalancer();
serviceCollection.AddSingleton<IHostedService, DiscoveryClientService>();
return serviceCollection;
services.TryAddTransient<DiscoveryHttpMessageHandler>();
services.AddSingleton<IServiceInstanceProvider>(p => p.GetService<IDiscoveryClient>());
services.AddHttpClient("DiscoveryRandom").AddRandomLoadBalancer();
services.AddHttpClient("DiscoveryRoundRobin").AddRoundRobinLoadBalancer();
services.AddSingleton<IHostedService, DiscoveryClientService>();
return services;
}

/// <summary>
Expand All @@ -168,6 +192,8 @@ public static IServiceCollection AddServiceDiscovery(this IServiceCollection ser
/// </exception>
public static IServiceInfo GetNamedDiscoveryServiceInfo(IConfiguration configuration, string serviceName)
{
ArgumentGuard.NotNull(configuration);

IServiceInfo info = configuration.GetServiceInfo(serviceName);

if (info == null)
Expand All @@ -194,36 +220,36 @@ public static IServiceInfo GetNamedDiscoveryServiceInfo(IConfiguration configura
/// </exception>
public static IServiceInfo GetSingletonDiscoveryServiceInfo(IConfiguration configuration)
{
ArgumentGuard.NotNull(configuration);

// Note: Could be other discovery type services in future
IEnumerable<EurekaServiceInfo> eurekaInfos = configuration.GetServiceInfos<EurekaServiceInfo>();
EurekaServiceInfo[] eurekaInfos = configuration.GetServiceInfos<EurekaServiceInfo>().ToArray();

if (eurekaInfos.Any())
{
if (eurekaInfos.Count() != 1)
if (eurekaInfos.Length != 1)
{
throw new ConnectorException("Multiple discovery service types bound to application.");
}

return eurekaInfos.First();
return eurekaInfos[0];
}

return null;
}

private static void ApplyDiscoveryOptions(IServiceCollection serviceCollection, Action<DiscoveryClientBuilder> builderAction)
private static void ApplyDiscoveryOptions(IServiceCollection services, IConfiguration configuration, Action<DiscoveryClientBuilder> builderAction)
{
var builder = new DiscoveryClientBuilder();

builderAction?.Invoke(builder);

if (builder.Extensions.Count > 1)
{
var configuration = serviceCollection.BuildServiceProvider().GetRequiredService<IConfiguration>();

IEnumerable<IDiscoveryClientExtension> configured =
builder.Extensions.Where(ext => ext.IsConfigured(configuration, GetSingletonDiscoveryServiceInfo(configuration)));
IDiscoveryClientExtension[] configured = builder.Extensions
.Where(extension => extension.IsConfigured(configuration, GetSingletonDiscoveryServiceInfo(configuration))).ToArray();

if (!configured.Any() || configured.Count() > 1)
if (!configured.Any() || configured.Length > 1)
{
throw new AmbiguousMatchException(
"Multiple IDiscoveryClient implementations have been registered and 0 or more than 1 have been configured! This is not supported, please only use a single client type.");
Expand All @@ -234,7 +260,7 @@ private static void ApplyDiscoveryOptions(IServiceCollection serviceCollection,

foreach (IDiscoveryClientExtension ext in builder.Extensions)
{
ext.ApplyServices(serviceCollection);
ext.ApplyServices(services);
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/Discovery/src/Client/DiscoveryWebApplicationBuilderExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public static WebApplicationBuilder AddDiscoveryClient(this WebApplicationBuilde
/// </exception>
public static WebApplicationBuilder AddServiceDiscovery(this WebApplicationBuilder hostBuilder, Action<DiscoveryClientBuilder> optionsAction)
{
hostBuilder.Services.AddServiceDiscovery(optionsAction);
hostBuilder.Services.AddServiceDiscovery(hostBuilder.Configuration, optionsAction);
return hostBuilder;
}
}
4 changes: 2 additions & 2 deletions src/Discovery/src/Client/DiscoveryWebHostBuilderExtensions.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public static class DiscoveryWebHostBuilderExtensions
/// </exception>
public static IWebHostBuilder AddDiscoveryClient(this IWebHostBuilder hostBuilder)
{
return hostBuilder.ConfigureServices(collection => collection.AddDiscoveryClient());
return hostBuilder.ConfigureServices((context, services) => services.AddDiscoveryClient(context.Configuration));
}

/// <summary>
Expand All @@ -55,6 +55,6 @@ public static IWebHostBuilder AddDiscoveryClient(this IWebHostBuilder hostBuilde
/// </exception>
public static IWebHostBuilder AddServiceDiscovery(this IWebHostBuilder hostBuilder, Action<DiscoveryClientBuilder> optionsAction)
{
return hostBuilder.ConfigureServices(collection => collection.AddServiceDiscovery(optionsAction));
return hostBuilder.ConfigureServices((context, services) => services.AddServiceDiscovery(context.Configuration, optionsAction));
}
}
Loading

0 comments on commit e934355

Please sign in to comment.