From aa621f687eb8e019ea7773c26758f0ebef901d9b Mon Sep 17 00:00:00 2001 From: Stephan Date: Tue, 13 Nov 2018 10:07:30 +0100 Subject: [PATCH] Refactor/fix registering controllers --- src/Umbraco.Core/Composing/TypeFinder.cs | 96 ++++++++++--------- src/Umbraco.Core/Composing/TypeLoader.cs | 9 ++ src/Umbraco.Web/LightInjectExtensions.cs | 93 ++++++++++++------ .../Runtime/WebRuntimeComponent.cs | 3 +- 4 files changed, 122 insertions(+), 79 deletions(-) diff --git a/src/Umbraco.Core/Composing/TypeFinder.cs b/src/Umbraco.Core/Composing/TypeFinder.cs index a42b84e0c5..9604f599cf 100644 --- a/src/Umbraco.Core/Composing/TypeFinder.cs +++ b/src/Umbraco.Core/Composing/TypeFinder.cs @@ -210,53 +210,55 @@ namespace Umbraco.Core.Composing /// NOTE the comma vs period... comma delimits the name in an Assembly FullName property so if it ends with comma then its an exact name match /// NOTE this means that "foo." will NOT exclude "foo.dll" but only "foo.*.dll" /// - internal static readonly string[] KnownAssemblyExclusionFilter = new[] - { - "mscorlib,", - "System.", - "Antlr3.", - "Autofac.", - "Autofac,", - "Castle.", - "ClientDependency.", - "DataAnnotationsExtensions.", - "DataAnnotationsExtensions,", - "Dynamic,", - "HtmlDiff,", - "Iesi.Collections,", - "Microsoft.", - "Newtonsoft.", - "NHibernate.", - "NHibernate,", - "NuGet.", - "RouteDebugger,", - "SqlCE4Umbraco,", - "umbraco.datalayer,", - "umbraco.interfaces,", - //"umbraco.providers,", - //"Umbraco.Web.UI,", - "umbraco.webservices", - "Lucene.", - "Examine,", - "Examine.", - "ServiceStack.", - "MySql.", - "HtmlAgilityPack.", - "TidyNet.", - "ICSharpCode.", - "CookComputing.", - "AutoMapper,", - "AutoMapper.", - "AzureDirectory,", - "itextsharp,", - "UrlRewritingNet.", - "HtmlAgilityPack,", - "MiniProfiler,", - "Moq,", - "nunit.framework,", - "TidyNet,", - "WebDriver," - }; + internal static readonly string[] KnownAssemblyExclusionFilter = { + "Antlr3.", + "AutoMapper,", + "AutoMapper.", + "Autofac,", // DI + "Autofac.", + "AzureDirectory,", + "Castle.", // DI, tests + "ClientDependency.", + "CookComputing.", + "CSharpTest.", // BTree for NuCache + "DataAnnotationsExtensions,", + "DataAnnotationsExtensions.", + "Dynamic,", + "Examine,", + "Examine.", + "HtmlAgilityPack,", + "HtmlAgilityPack.", + "HtmlDiff,", + "ICSharpCode.", + "Iesi.Collections,", // used by NHibernate + "LightInject.", // DI + "LightInject,", + "Lucene.", + "Markdown,", + "Microsoft.", + "MiniProfiler,", + "Moq,", + "MySql.", + "NHibernate,", + "NHibernate.", + "Newtonsoft.", + "NPoco,", + "NuGet.", + "RouteDebugger,", + "Semver.", + "Serilog.", + "Serilog,", + "ServiceStack.", + "SqlCE4Umbraco,", + "Superpower,", // used by Serilog + "System.", + "TidyNet,", + "TidyNet.", + "WebDriver,", + "itextsharp,", + "mscorlib,", + "nunit.framework,", + }; /// /// Finds any classes derived from the type T that contain the attribute TAttribute diff --git a/src/Umbraco.Core/Composing/TypeLoader.cs b/src/Umbraco.Core/Composing/TypeLoader.cs index 304638e017..f79c288e91 100644 --- a/src/Umbraco.Core/Composing/TypeLoader.cs +++ b/src/Umbraco.Core/Composing/TypeLoader.cs @@ -520,6 +520,8 @@ namespace Umbraco.Core.Composing // if not caching, or not IDiscoverable, directly get types if (cache == false || typeof(IDiscoverable).IsAssignableFrom(typeof(T)) == false) { + _logger.Logger.Debug("Running a full, non-cached, scan for type {TypeName} (slow).", typeof(T).FullName); + return GetTypesInternal( typeof (T), null, () => TypeFinder.FindClassesOfType(specificAssemblies ?? AssembliesToScan), @@ -559,6 +561,8 @@ namespace Umbraco.Core.Composing // if not caching, or not IDiscoverable, directly get types if (cache == false || typeof(IDiscoverable).IsAssignableFrom(typeof(T)) == false) { + _logger.Logger.Debug("Running a full, non-cached, scan for type {TypeName} / attribute {AttributeName} (slow).", typeof(T).FullName, typeof(TAttribute).FullName); + return GetTypesInternal( typeof (T), typeof (TAttribute), () => TypeFinder.FindClassesOfTypeWithAttribute(specificAssemblies ?? AssembliesToScan), @@ -595,6 +599,11 @@ namespace Umbraco.Core.Composing // do not cache anything from specific assemblies cache &= specificAssemblies == null; + if (cache == false) + { + _logger.Logger.Debug("Running a full, non-cached, scan for types / attribute {AttributeName} (slow).", typeof(TAttribute).FullName); + } + return GetTypesInternal( typeof (object), typeof (TAttribute), () => TypeFinder.FindClassesWithAttribute(specificAssemblies ?? AssembliesToScan), diff --git a/src/Umbraco.Web/LightInjectExtensions.cs b/src/Umbraco.Web/LightInjectExtensions.cs index 580dc4117d..7ea55587e0 100644 --- a/src/Umbraco.Web/LightInjectExtensions.cs +++ b/src/Umbraco.Web/LightInjectExtensions.cs @@ -1,49 +1,82 @@ -using System.Reflection; +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; using System.Web.Http.Controllers; using System.Web.Mvc; using LightInject; -using Umbraco.Core; using Umbraco.Core.Composing; +using Umbraco.Web.Mvc; +using Umbraco.Web.WebApi; namespace Umbraco.Web { internal static class LightInjectExtensions { /// - /// Registers all IControllers using the TypeLoader for scanning and caching found instances for the calling assembly + /// Registers Umbraco controllers. /// - /// - /// - /// - public static void RegisterMvcControllers(this IServiceRegistry container, TypeLoader typeLoader, Assembly assembly) + public static void RegisterUmbracoControllers(this IServiceRegistry container, TypeLoader typeLoader, Assembly umbracoWebAssembly) { - //TODO: We've already scanned for UmbracoApiControllers and SurfaceControllers - should we scan again - // for all controllers? Seems like we should just do this once and then filter. That said here we are - // only scanning our own single assembly. Hrm. + // notes + // + // We scan and auto-registers: + // - every IController and IHttpController that *we* have in Umbraco.Web + // - PluginController and UmbracoApiController in every assembly + // + // We do NOT scan: + // - any IController or IHttpController (anything not PluginController nor UmbracoApiController), outside of Umbraco.Web + // which means that users HAVE to explicitly register their own non-Umbraco controllers + // + // This is because we try to achieve a balance between "simple" and "fast. Scanning for PluginController or + // UmbracoApiController is fast-ish because they both are IDiscoverable. Scanning for IController or IHttpController + // is a full, non-cached scan = expensive, we do it only for 1 assembly. + // + // TODO + // find a way to scan for IController *and* IHttpController in one single pass + // or, actually register them manually so don't require a full scan for these + // 5 are IController but not PluginController + // Umbraco.Web.Mvc.RenderMvcController + // Umbraco.Web.Install.Controllers.InstallController + // Umbraco.Web.Macros.PartialViewMacroController + // Umbraco.Web.Editors.PreviewController + // Umbraco.Web.Editors.BackOfficeController + // 9 are IHttpController but not UmbracoApiController + // Umbraco.Web.Controllers.UmbProfileController + // Umbraco.Web.Controllers.UmbLoginStatusController + // Umbraco.Web.Controllers.UmbRegisterController + // Umbraco.Web.Controllers.UmbLoginController + // Umbraco.Web.Mvc.RenderMvcController + // Umbraco.Web.Install.Controllers.InstallController + // Umbraco.Web.Macros.PartialViewMacroController + // Umbraco.Web.Editors.PreviewController + // Umbraco.Web.Editors.BackOfficeController - container.RegisterControllers(typeLoader, assembly); + // scan and register every IController in Umbraco.Web + var umbracoWebControllers = typeLoader.GetTypes(specificAssemblies: new[] { umbracoWebAssembly }); + //foreach (var controller in umbracoWebControllers.Where(x => !typeof(PluginController).IsAssignableFrom(x))) + // Current.Logger.Debug(typeof(LightInjectExtensions), "IController NOT PluginController: " + controller.FullName); + container.RegisterControllers(umbracoWebControllers); + + // scan and register every PluginController in everything (PluginController is IDiscoverable and IController) + var nonUmbracoWebPluginController = typeLoader.GetTypes().Where(x => x.Assembly != umbracoWebAssembly); + container.RegisterControllers(nonUmbracoWebPluginController); + + // scan and register every IHttpController in Umbraco.Web + var umbracoWebHttpControllers = typeLoader.GetTypes(specificAssemblies: new[] { umbracoWebAssembly }); + //foreach (var controller in umbracoWebControllers.Where(x => !typeof(UmbracoApiController).IsAssignableFrom(x))) + // Current.Logger.Debug(typeof(LightInjectExtensions), "IHttpController NOT UmbracoApiController: " + controller.FullName); + container.RegisterControllers(umbracoWebHttpControllers); + + // scan and register every UmbracoApiController in everything (UmbracoApiController is IDiscoverable and IHttpController) + var nonUmbracoWebApiControllers = typeLoader.GetTypes().Where(x => x.Assembly != umbracoWebAssembly); + container.RegisterControllers(nonUmbracoWebApiControllers); } - /// - /// Registers all IHttpController using the TypeLoader for scanning and caching found instances for the calling assembly - /// - /// - /// - /// - public static void RegisterApiControllers(this IServiceRegistry container, TypeLoader typeLoader, Assembly assembly) + private static void RegisterControllers(this IServiceRegistry container, IEnumerable controllerTypes) { - //TODO: We've already scanned for UmbracoApiControllers and SurfaceControllers - should we scan again - // for all controllers? Seems like we should just do this once and then filter. That said here we are - // only scanning our own single assembly. Hrm. - - container.RegisterControllers(typeLoader, assembly); - } - - private static void RegisterControllers(this IServiceRegistry container, TypeLoader typeLoader, Assembly assembly) - { - var types = typeLoader.GetTypes(specificAssemblies: new[] { assembly }); - foreach (var type in types) - container.Register(type, new PerRequestLifeTime()); + foreach (var controllerType in controllerTypes) + container.Register(controllerType, new PerRequestLifeTime()); } } } diff --git a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs index 1af24db636..bd09cbe918 100644 --- a/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs +++ b/src/Umbraco.Web/Runtime/WebRuntimeComponent.cs @@ -122,9 +122,8 @@ namespace Umbraco.Web.Runtime composition.Container.EnableMvc(); // does container.EnablePerWebRequestScope() composition.Container.ScopeManagerProvider = smp; // reverts - we will do it last (in WebRuntime) - composition.Container.RegisterMvcControllers(typeLoader, GetType().Assembly); + composition.Container.RegisterUmbracoControllers(typeLoader, GetType().Assembly); composition.Container.EnableWebApi(GlobalConfiguration.Configuration); - composition.Container.RegisterApiControllers(typeLoader, GetType().Assembly); composition.Container.RegisterCollectionBuilder() .Add(() => typeLoader.GetTypes()); // fixme which searchable trees?!