From 77ce21b0e32931cc99d9fbfa94a4d66d5f070bee Mon Sep 17 00:00:00 2001 From: Shannon Date: Wed, 25 Mar 2020 17:35:43 +1100 Subject: [PATCH] Adds notes/test about pre building the netcore container --- src/Umbraco.Tests.Integration/ContainerTests.cs | 8 ++++++++ .../AspNetCore/UmbracoCoreServiceCollectionExtensions.cs | 6 +++--- .../UmbracoWebsiteServiceCollectionExtensions.cs | 1 + 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/Umbraco.Tests.Integration/ContainerTests.cs b/src/Umbraco.Tests.Integration/ContainerTests.cs index b439e32df1..89cf2b5f7f 100644 --- a/src/Umbraco.Tests.Integration/ContainerTests.cs +++ b/src/Umbraco.Tests.Integration/ContainerTests.cs @@ -73,6 +73,14 @@ namespace Umbraco.Tests.Integration [Test] public async Task BuildServiceProvider_Before_Host_Is_Configured() { + // This is a test to show an anti-pattern used in netcore. This should be avoided in all cases if possible. + // There's a thread about this here: https://github.com/dotnet/aspnetcore/issues/14587 + // For some reason we are not being warned about this with our code analysis since we are using it + // in a couple of places but we should really try to see if we can avoid it. + // The test below shows how it could be possible to resolve an instance and then re-register it as a factory + // so that only one singleton instance is every created, but it's hacky and like Fowler says in that article + // it means the container won't be disposed, and maybe other services? not sure. + var umbracoContainer = RuntimeTests.GetUmbracoContainer(out var serviceProviderFactory); IHostApplicationLifetime lifetime1 = null; diff --git a/src/Umbraco.Web.BackOffice/AspNetCore/UmbracoCoreServiceCollectionExtensions.cs b/src/Umbraco.Web.BackOffice/AspNetCore/UmbracoCoreServiceCollectionExtensions.cs index d283a7ffb1..20901822e3 100644 --- a/src/Umbraco.Web.BackOffice/AspNetCore/UmbracoCoreServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.BackOffice/AspNetCore/UmbracoCoreServiceCollectionExtensions.cs @@ -29,6 +29,7 @@ namespace Umbraco.Web.BackOffice.AspNetCore /// public static IServiceCollection AddUmbracoConfiguration(this IServiceCollection services) { + // TODO: We need to avoid this, surely there's a way? See ContainerTests.BuildServiceProvider_Before_Host_Is_Configured var serviceProvider = services.BuildServiceProvider(); var configuration = serviceProvider.GetService(); if (configuration == null) @@ -129,8 +130,7 @@ namespace Umbraco.Web.BackOffice.AspNetCore out ILogger logger, out Configs configs, out IIOHelper ioHelper, out Core.Hosting.IHostingEnvironment hostingEnvironment, out IBackOfficeInfo backOfficeInfo, out IProfiler profiler) { - // TODO: Resolving services before the Host is done configuring this way means that the services resolved - // are not going to be the same instances that are going to be used within the application! + // TODO: We need to avoid this, surely there's a way? See ContainerTests.BuildServiceProvider_Before_Host_Is_Configured var serviceProvider = services.BuildServiceProvider(); var httpContextAccessor = serviceProvider.GetRequiredService(); @@ -148,7 +148,7 @@ namespace Umbraco.Web.BackOffice.AspNetCore ioHelper = new IOHelper(hostingEnvironment, globalSettings); logger = SerilogLogger.CreateWithDefaultConfiguration(hostingEnvironment, new AspNetCoreSessionIdResolver(httpContextAccessor), - // need to build a new service provider since the one already resolved above doesn't have the IRequestCache yet + // TODO: We need to avoid this, surely there's a way? See ContainerTests.BuildServiceProvider_Before_Host_Is_Configured () => services.BuildServiceProvider().GetService(), coreDebug, ioHelper, new AspNetCoreMarchal()); diff --git a/src/Umbraco.Web.Website/AspNetCore/UmbracoWebsiteServiceCollectionExtensions.cs b/src/Umbraco.Web.Website/AspNetCore/UmbracoWebsiteServiceCollectionExtensions.cs index f7a198bc3b..6d2ce5e7e4 100644 --- a/src/Umbraco.Web.Website/AspNetCore/UmbracoWebsiteServiceCollectionExtensions.cs +++ b/src/Umbraco.Web.Website/AspNetCore/UmbracoWebsiteServiceCollectionExtensions.cs @@ -19,6 +19,7 @@ namespace Umbraco.Web.Website.AspNetCore { public static IServiceCollection AddUmbracoWebsite(this IServiceCollection services) { + // TODO: We need to avoid this, surely there's a way? See ContainerTests.BuildServiceProvider_Before_Host_Is_Configured var serviceProvider = services.BuildServiceProvider(); var configs = serviceProvider.GetService(); var imagingSettings = configs.Imaging();