Only add default services once in WebApplicationBuilder #34782
Conversation
ghost
added
the
halter73
changed the title
| [Fact] | ||
| public void WebApplicationBuilder_EnablesServiceScopeValidationByDefaultInDevelopment() | ||
| { | ||
| // The environment cannot be reconfigured after the builder is created currently. |
There was a problem hiding this comment.
Not part of this PR but we should talk about ways to throw if UseEnvironment is used after this call (and maybe we need a first class API).
| CopyPropertiesTo(context.HostingEnvironment); | ||
| }); | ||
| var context = (WebHostBuilderContext)hostBuilder.Properties[typeof(WebHostBuilderContext)]; | ||
| CopyPropertiesTo(context.HostingEnvironment); |
There was a problem hiding this comment.
If not, we get test failures here and other similar places since WebApplication.Environment pulls from the IServiceProvider and we no longer register our IWebHostEnvironment as a service.
|
I debugged the issue, the IWebHostEnvironment.ApplicationName is incorrect and it results in not being able to find controllers or views. |
| } | ||
|
|
||
| _environment.ApplyConfigurationSettings(_configuration); | ||
|
|
There was a problem hiding this comment.
Why do we ApplyConfigurationSettings twice when nothing is observing changes to _environment between these 2 calls.?
There was a problem hiding this comment.
This isn't new. ConfigureHostConfiguration callbacks to read environment variables (e.g. DOTNET_ and ASPNETCORE_) that need to be reacted to before running ConfigureAppConfiguration callbacks to read config that's dependent on the environment name configured by the environment variables (e.g. appsettings.{env.EnvironmentName}.json and user secrets).
This is the last call to ApplyConfigurationSettings before the WebApplicationBuilder ctor exits, so it's to ensure app code in Program.cs can observe any changes made to the environment in appsettings mostly.
There was a problem hiding this comment.
After playing with this a bit I think we only need to read hosting configuration once at the beginning and blocking it after that. We should also get rid of our custom IWebHostEnvironment and should use the one configured in the IHostBuilder.Properties[typeof(WebHostContext)]
| // Ideally, we'd only call _hostBuilder.ConfigureWebHost(ConfigureWebHost) instead of | ||
| // _hostBuilder.ConfigureWebHostDefaults(ConfigureWebHost) to avoid some duplicate service descriptors, | ||
| // but we want to add services in the WebApplicationBuilder constructor so code can inspect | ||
| // WebApplicationBuilder.Services. At the same time, we want to be able which services are loaded | ||
| // to react to config changes (e.g. ForwardedHeadersStartupFilter). |
There was a problem hiding this comment.
| // Ideally, we'd only call _hostBuilder.ConfigureWebHost(ConfigureWebHost) instead of | |
| // _hostBuilder.ConfigureWebHostDefaults(ConfigureWebHost) to avoid some duplicate service descriptors, | |
| // but we want to add services in the WebApplicationBuilder constructor so code can inspect | |
| // WebApplicationBuilder.Services. At the same time, we want to be able which services are loaded | |
| // to react to config changes (e.g. ForwardedHeadersStartupFilter). |
|
This PR gave me clarity on how we should fix the remaining issues with the WebApplicationBuilder. |
amcasey
added
area-minimal
Fixes #33934