Skip to content

Commit 20f4180

Browse files
halter73halter73
committed
Only add default services once
1 parent c2694e4 commit 20f4180

File tree

4 files changed

+126
-78
lines changed

4 files changed

+126
-78
lines changed

‎src/DefaultBuilder/src/BootstrapHostBuilder.cs‎

Lines changed: 47 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
5-
using System.Collections.Generic;
64
using Microsoft.AspNetCore.Builder;
75
using Microsoft.Extensions.Configuration;
86
using Microsoft.Extensions.DependencyInjection;
@@ -15,16 +13,20 @@ internal class BootstrapHostBuilder : IHostBuilder
1513
{
1614
private readonly ConfigurationManager _configuration;
1715
private readonly WebHostEnvironment _environment;
18-
16+
private readonly IServiceCollection _serviceCollection;
1917
private readonly HostBuilderContext _hostContext;
2018

2119
private readonly List<Action<IConfigurationBuilder>> _configureHostActions = new();
2220
private readonly List<Action<HostBuilderContext, IConfigurationBuilder>> _configureAppActions = new();
21+
private readonly List<Action<HostBuilderContext, IServiceCollection>> _configureServicesActions = new();
22+
23+
private readonly List<Action<IHostBuilder>> _delayUntilBuildActions = new();
2324

24-
public BootstrapHostBuilder(ConfigurationManager configuration, WebHostEnvironment webHostEnvironment)
25+
public BootstrapHostBuilder(ConfigurationManager configuration, WebHostEnvironment webHostEnvironment, IServiceCollection serviceCollection)
2526
{
2627
_configuration = configuration;
2728
_environment = webHostEnvironment;
29+
_serviceCollection = serviceCollection;
2830

2931
_hostContext = new HostBuilderContext(Properties)
3032
{
@@ -51,6 +53,12 @@ public IHostBuilder ConfigureContainer<TContainerBuilder>(Action<HostBuilderCont
5153
{
5254
// This is not called by HostingHostBuilderExtensions.ConfigureDefaults currently, but that could change in the future.
5355
// If this does get called in the future, it should be called again at a later stage on the ConfigureHostBuilder.
56+
if (configureDelegate is null)
57+
{
58+
throw new ArgumentNullException(nameof(configureDelegate));
59+
}
60+
61+
_delayUntilBuildActions.Add(hostBuilder => hostBuilder.ConfigureContainer<TContainerBuilder>(configureDelegate));
5462
return this;
5563
}
5664

@@ -68,30 +76,43 @@ public IHostBuilder ConfigureHostConfiguration(Action<IConfigurationBuilder> con
6876
public IHostBuilder ConfigureServices(Action<HostBuilderContext, IServiceCollection> configureDelegate)
6977
{
7078
// HostingHostBuilderExtensions.ConfigureDefaults calls this via ConfigureLogging
71-
// during the initial config stage. It should be called again later on the ConfigureHostBuilder.
79+
_configureServicesActions.Add(configureDelegate ?? throw new ArgumentNullException(nameof(configureDelegate)));
7280
return this;
7381
}
7482

7583
public IHostBuilder UseServiceProviderFactory<TContainerBuilder>(IServiceProviderFactory<TContainerBuilder> factory) where TContainerBuilder : notnull
7684
{
7785
// This is not called by HostingHostBuilderExtensions.ConfigureDefaults currently, but that could change in the future.
7886
// If this does get called in the future, it should be called again at a later stage on the ConfigureHostBuilder.
87+
if (factory is null)
88+
{
89+
throw new ArgumentNullException(nameof(factory));
90+
}
91+
92+
_delayUntilBuildActions.Add(hostBuilder => hostBuilder.UseServiceProviderFactory<TContainerBuilder>(factory));
7993
return this;
8094
}
8195

8296
public IHostBuilder UseServiceProviderFactory<TContainerBuilder>(Func<HostBuilderContext, IServiceProviderFactory<TContainerBuilder>> factory) where TContainerBuilder : notnull
8397
{
8498
// HostingHostBuilderExtensions.ConfigureDefaults calls this via UseDefaultServiceProvider
8599
// during the initial config stage. It should be called again later on the ConfigureHostBuilder.
100+
if (factory is null)
101+
{
102+
throw new ArgumentNullException(nameof(factory));
103+
}
104+
105+
_delayUntilBuildActions.Add(hostBuilder => hostBuilder.UseServiceProviderFactory<TContainerBuilder>(factory));
86106
return this;
87107
}
88108

89-
internal void RunConfigurationCallbacks()
109+
public void RunDefaultCallbacks()
90110
{
91111
foreach (var configureHostAction in _configureHostActions)
92112
{
93113
configureHostAction(_configuration);
94114
}
115+
_configureHostActions.Clear();
95116

96117
// Configuration doesn't auto-update during the bootstrap phase to reduce I/O,
97118
// but we do need to update between host and app configuration so the right environment is used.
@@ -101,8 +122,28 @@ internal void RunConfigurationCallbacks()
101122
{
102123
configureAppAction(_hostContext, _configuration);
103124
}
125+
_configureAppActions.Clear();
104126

105127
_environment.ApplyConfigurationSettings(_configuration);
128+
129+
foreach (var configureServicesAction in _configureServicesActions)
130+
{
131+
configureServicesAction(_hostContext, _serviceCollection);
132+
}
133+
_configureServicesActions.Clear();
134+
}
135+
136+
public void RunDelayedCallbacks(IHostBuilder innerBuilder)
137+
{
138+
// This will only run callbacks added by WebApplicationBuilder.Build().
139+
// Other callbacks were run and cleared in the WebApplicationBuilder ctor.
140+
RunDefaultCallbacks();
141+
142+
foreach (var callback in _delayUntilBuildActions)
143+
{
144+
callback(innerBuilder);
145+
}
146+
_delayUntilBuildActions.Clear();
106147
}
107148
}
108149
}

‎src/DefaultBuilder/src/ConfigureHostBuilder.cs‎

Lines changed: 6 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,6 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4-
using System;
5-
using System.Collections.Generic;
64
using Microsoft.Extensions.Configuration;
75
using Microsoft.Extensions.DependencyInjection;
86
using Microsoft.Extensions.Hosting;
@@ -39,8 +37,6 @@ internal ConfigureHostBuilder(ConfigurationManager configuration, WebHostEnviron
3937
};
4038
}
4139

42-
internal bool ConfigurationEnabled { get; set; }
43-
4440
IHost IHostBuilder.Build()
4541
{
4642
throw new NotSupportedException($"Call {nameof(WebApplicationBuilder)}.{nameof(WebApplicationBuilder.Build)}() instead.");
@@ -49,12 +45,9 @@ IHost IHostBuilder.Build()
4945
/// <inheritdoc />
5046
public IHostBuilder ConfigureAppConfiguration(Action<HostBuilderContext, IConfigurationBuilder> configureDelegate)
5147
{
52-
if (ConfigurationEnabled)
53-
{
54-
// Run these immediately so that they are observable by the imperative code
55-
configureDelegate(_context, _configuration);
56-
_environment.ApplyConfigurationSettings(_configuration);
57-
}
48+
// Run these immediately so that they are observable by the imperative code
49+
configureDelegate(_context, _configuration);
50+
_environment.ApplyConfigurationSettings(_configuration);
5851

5952
return this;
6053
}
@@ -74,12 +67,9 @@ public IHostBuilder ConfigureContainer<TContainerBuilder>(Action<HostBuilderCont
7467
/// <inheritdoc />
7568
public IHostBuilder ConfigureHostConfiguration(Action<IConfigurationBuilder> configureDelegate)
7669
{
77-
if (ConfigurationEnabled)
78-
{
79-
// Run these immediately so that they are observable by the imperative code
80-
configureDelegate(_configuration);
81-
_environment.ApplyConfigurationSettings(_configuration);
82-
}
70+
// Run these immediately so that they are observable by the imperative code
71+
configureDelegate(_configuration);
72+
_environment.ApplyConfigurationSettings(_configuration);
8373

8474
return this;
8575
}

‎src/DefaultBuilder/src/WebApplicationBuilder.cs‎

Lines changed: 52 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,17 @@ public sealed class WebApplicationBuilder
2020
private const string EndpointRouteBuilderKey = "__EndpointRouteBuilder";
2121

2222
private readonly HostBuilder _hostBuilder = new();
23+
24+
private readonly BootstrapHostBuilder _bootstrapHostBuilder;
2325
private readonly ConfigureHostBuilder _deferredHostBuilder;
2426
private readonly ConfigureWebHostBuilder _deferredWebHostBuilder;
27+
2528
private readonly WebHostEnvironment _environment;
2629
private readonly WebApplicationServiceCollection _services = new();
30+
31+
// This is effectively readonly because it is always set inline by the ConfigureWebHostDefaults callback in the ctor.
32+
private IWebHostBuilder _bootstrapWebHostBuilder = default!;
33+
2734
private WebApplication? _builtApplication;
2835

2936
internal WebApplicationBuilder(Assembly? callingAssembly, string[]? args = null)
@@ -34,31 +41,27 @@ internal WebApplicationBuilder(Assembly? callingAssembly, string[]? args = null)
3441
Environment = _environment = new WebHostEnvironment(callingAssembly);
3542

3643
Configuration.SetBasePath(_environment.ContentRootPath);
37-
Services.AddSingleton(Environment);
3844

3945
// Run methods to configure both generic and web host defaults early to populate config from appsettings.json
4046
// environment variables (both DOTNET_ and ASPNETCORE_ prefixed) and other possible default sources to prepopulate
4147
// the correct defaults.
42-
var bootstrapBuilder = new BootstrapHostBuilder(Configuration, _environment);
43-
bootstrapBuilder.ConfigureDefaults(args);
44-
bootstrapBuilder.ConfigureWebHostDefaults(configure: _ => { });
45-
bootstrapBuilder.RunConfigurationCallbacks();
48+
_bootstrapHostBuilder = new BootstrapHostBuilder(Configuration, _environment, Services);
49+
_bootstrapHostBuilder.ConfigureDefaults(args);
50+
_bootstrapHostBuilder.ConfigureWebHostDefaults(webHostBuilder =>
51+
{
52+
// This runs inline.
53+
_bootstrapWebHostBuilder = webHostBuilder;
54+
});
55+
_bootstrapHostBuilder.RunDefaultCallbacks();
4656

4757
Logging = new LoggingBuilder(Services);
4858
WebHost = _deferredWebHostBuilder = new ConfigureWebHostBuilder(Configuration, _environment, Services);
4959
Host = _deferredHostBuilder = new ConfigureHostBuilder(Configuration, _environment, Services);
5060

51-
// Add default services
52-
_deferredHostBuilder.ConfigureDefaults(args);
53-
54-
// Enable changes here because we need to pick up configuration sources added by the generic web host
55-
_deferredHostBuilder.ConfigurationEnabled = true;
56-
_deferredHostBuilder.ConfigureWebHostDefaults(configure: _ => { });
57-
5861
// This is important because GenericWebHostBuilder does the following and we want to preserve the WebHostBuilderContext:
5962
// context.Properties[typeof(WebHostBuilderContext)] = webHostBuilderContext;
6063
// context.Properties[typeof(WebHostOptions)] = options;
61-
foreach (var (key, value) in _deferredHostBuilder.Properties)
64+
foreach (var (key, value) in _bootstrapHostBuilder.Properties)
6265
{
6366
_hostBuilder.Properties[key] = value;
6467
}
@@ -116,10 +119,42 @@ public WebApplication Build()
116119
}
117120
});
118121

119-
// We call ConfigureWebHostDefaults AGAIN because config might be added like "ForwardedHeaders_Enabled"
120-
// which can add even more services. If not for that, we probably call _hostBuilder.ConfigureWebHost(ConfigureWebHost)
121-
// instead in order to avoid duplicate service registration.
122-
_hostBuilder.ConfigureWebHostDefaults(ConfigureWebHost);
122+
// Configure the original GenericWebHostBuilder that added the default services we use.
123+
_bootstrapWebHostBuilder.Configure(ConfigureApplication);
124+
_environment.ApplyEnvironmentSettings(_bootstrapWebHostBuilder);
125+
// This has further modified the _bootstrapHostBuilder, so we need to run the newly added callbacks in the proper order.
126+
_bootstrapHostBuilder.RunDelayedCallbacks(_hostBuilder);
127+
128+
// This needs to go here to avoid adding the IHostedService that boots the server twice (the GenericWebHostService).
129+
// Copy the services that were added via WebApplicationBuilder.Services into the final IServiceCollection
130+
_hostBuilder.ConfigureServices((context, services) =>
131+
{
132+
// We've only added services configured by the GenericWebHostBuilder and WebHost.ConfigureWebDefaults
133+
// at this point. HostBuilder news up a new ServiceCollection in HostBuilder.Build() we haven't seen
134+
// until now, so we cannot clear these services even though some are redundant because
135+
// we called ConfigureWebHostDefaults on both the _deferredHostBuilder and _hostBuilder.
136+
137+
// Ideally, we'd only call _hostBuilder.ConfigureWebHost(ConfigureWebHost) instead of
138+
// _hostBuilder.ConfigureWebHostDefaults(ConfigureWebHost) to avoid some duplicate service descriptors,
139+
// but we want to add services in the WebApplicationBuilder constructor so code can inspect
140+
// WebApplicationBuilder.Services. At the same time, we want to be able which services are loaded
141+
// to react to config changes (e.g. ForwardedHeadersStartupFilter).
142+
foreach (var s in _services)
143+
{
144+
services.Add(s);
145+
}
146+
147+
// Add any services to the user visible service collection so that they are observable
148+
// just in case users capture the Services property. Orchard does this to get a "blueprint"
149+
// of the service collection
150+
151+
// Drop the reference to the existing collection and set the inner collection
152+
// to the new one. This allows code that has references to the service collection to still function.
153+
_services.InnerCollection = services;
154+
});
155+
156+
// Run the other callbacks on the final host builder
157+
_deferredHostBuilder.RunDeferredCallbacks(_hostBuilder);
123158

124159
_builtApplication = new WebApplication(_hostBuilder.Build());
125160

@@ -202,44 +237,6 @@ private void ConfigureApplication(WebHostBuilderContext context, IApplicationBui
202237
}
203238
}
204239

205-
private void ConfigureWebHost(IWebHostBuilder genericWebHostBuilder)
206-
{
207-
genericWebHostBuilder.Configure(ConfigureApplication);
208-
209-
// This needs to go here to avoid adding the IHostedService that boots the server twice (the GenericWebHostService).
210-
// Copy the services that were added via WebApplicationBuilder.Services into the final IServiceCollection
211-
genericWebHostBuilder.ConfigureServices((context, services) =>
212-
{
213-
// We've only added services configured by the GenericWebHostBuilder and WebHost.ConfigureWebDefaults
214-
// at this point. HostBuilder news up a new ServiceCollection in HostBuilder.Build() we haven't seen
215-
// until now, so we cannot clear these services even though some are redundant because
216-
// we called ConfigureWebHostDefaults on both the _deferredHostBuilder and _hostBuilder.
217-
218-
// Ideally, we'd only call _hostBuilder.ConfigureWebHost(ConfigureWebHost) instead of
219-
// _hostBuilder.ConfigureWebHostDefaults(ConfigureWebHost) to avoid some duplicate service descriptors,
220-
// but we want to add services in the WebApplicationBuilder constructor so code can inspect
221-
// WebApplicationBuilder.Services. At the same time, we want to be able which services are loaded
222-
// to react to config changes (e.g. ForwardedHeadersStartupFilter).
223-
foreach (var s in _services)
224-
{
225-
services.Add(s);
226-
}
227-
228-
// Add any services to the user visible service collection so that they are observable
229-
// just in case users capture the Services property. Orchard does this to get a "blueprint"
230-
// of the service collection
231-
232-
// Drop the reference to the existing collection and set the inner collection
233-
// to the new one. This allows code that has references to the service collection to still function.
234-
_services.InnerCollection = services;
235-
});
236-
237-
// Run the other callbacks on the final host builder
238-
_deferredHostBuilder.RunDeferredCallbacks(_hostBuilder);
239-
240-
_environment.ApplyEnvironmentSettings(genericWebHostBuilder);
241-
}
242-
243240
private static IEndpointRouteBuilder? GetEndpointRouteBuilder(IApplicationBuilder app)
244241
{
245242
app.Properties.TryGetValue(EndpointRouteBuilderKey, out var value);

‎src/DefaultBuilder/test/Microsoft.AspNetCore.Tests/WebApplicationTests.cs‎

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22
// The .NET Foundation licenses this file to you under the MIT license.
33

44
using System.Collections.Concurrent;
5-
using System.Collections.Generic;
65
using System.Diagnostics;
76
using System.Diagnostics.Tracing;
87
using Microsoft.AspNetCore.Builder;
@@ -18,6 +17,7 @@
1817
using Microsoft.Extensions.DependencyInjection;
1918
using Microsoft.Extensions.Hosting;
2019
using Microsoft.Extensions.Logging;
20+
using Microsoft.Extensions.Logging.Console;
2121
using Microsoft.Extensions.Options;
2222
using Xunit;
2323

@@ -533,6 +533,26 @@ public async Task WebApplicationBuilder_StartupFilterCanAddTerminalMiddleware()
533533
Assert.Equal(418, (int)terminalResult.StatusCode);
534534
}
535535

536+
[Fact]
537+
public async Task WebApplicationBuilder_OnlyAddsDefaultServicesOnce()
538+
{
539+
var builder = WebApplication.CreateBuilder();
540+
541+
// IWebHostEnvironment is added by ConfigureDefaults
542+
Assert.Single(builder.Services.Where(descriptor => descriptor.ServiceType == typeof(IConfigureOptions<LoggerFactoryOptions>)));
543+
// IWebHostEnvironment is added by ConfigureWebHostDefaults
544+
Assert.Single(builder.Services.Where(descriptor => descriptor.ServiceType == typeof(IWebHostEnvironment)));
545+
Assert.Single(builder.Services.Where(descriptor => descriptor.ServiceType == typeof(IOptionsChangeTokenSource<HostFilteringOptions>)));
546+
Assert.Single(builder.Services.Where(descriptor => descriptor.ServiceType == typeof(IServer)));
547+
548+
await using var app = builder.Build();
549+
550+
Assert.Single(app.Services.GetRequiredService<IEnumerable<IConfigureOptions<LoggerFactoryOptions>>>());
551+
Assert.Single(app.Services.GetRequiredService<IEnumerable<IWebHostEnvironment>>());
552+
Assert.Single(app.Services.GetRequiredService<IEnumerable<IOptionsChangeTokenSource<HostFilteringOptions>>>());
553+
Assert.Single(app.Services.GetRequiredService<IEnumerable<IServer>>());
554+
}
555+
536556
private class Service : IService { }
537557
private interface IService { }
538558

0 commit comments

Comments
 (0)