From b6a0969c1ceaabe83f67e79625f84d9afa3046b1 Mon Sep 17 00:00:00 2001 From: Yishai Galatzer Date: Fri, 7 Feb 2014 16:20:12 -0800 Subject: [PATCH] Support per process caching of controller discovery Scan only relevant (non skipped) assemblies FinalizeSetup point, to make "stuff" immutable Support controllers under any namespace support customizing controller discovery support customizing skipped assemblies support customizing multiple controllers with the same name under different namespaces and assemblies support controller ambiguity detection --- samples/MvcSample/Home2Controller.cs | 2 +- samples/MvcSample/HomeController.cs | 3 + samples/MvcSample/Startup.cs | 8 +- .../MvcHandler.cs | 10 +- .../MvcServices.cs | 120 ++++++++++++--- .../ActionInvokerProvider.cs | 9 +- .../ControllerActionInvoker.cs | 6 +- src/Microsoft.AspNet.Mvc/ControllerCache.cs | 9 ++ .../ControllerDescriptor.cs | 35 +++++ .../DefaultControllerCache.cs | 79 ++++++++++ .../DefaultControllerFactory.cs | 38 +++-- .../DefaultSkipAssemblies.cs | 141 ++++++++++++++++++ src/Microsoft.AspNet.Mvc/IFinalizeSetup.cs | 7 + src/Microsoft.AspNet.Mvc/SkipAssemblies.cs | 11 ++ src/Microsoft.AspNet.Mvc/SkipNoAssemblies.cs | 12 ++ 15 files changed, 438 insertions(+), 52 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc/ControllerCache.cs create mode 100644 src/Microsoft.AspNet.Mvc/ControllerDescriptor.cs create mode 100644 src/Microsoft.AspNet.Mvc/DefaultControllerCache.cs create mode 100644 src/Microsoft.AspNet.Mvc/DefaultSkipAssemblies.cs create mode 100644 src/Microsoft.AspNet.Mvc/IFinalizeSetup.cs create mode 100644 src/Microsoft.AspNet.Mvc/SkipAssemblies.cs create mode 100644 src/Microsoft.AspNet.Mvc/SkipNoAssemblies.cs diff --git a/samples/MvcSample/Home2Controller.cs b/samples/MvcSample/Home2Controller.cs index 59d84891aa..9ce16357e5 100644 --- a/samples/MvcSample/Home2Controller.cs +++ b/samples/MvcSample/Home2Controller.cs @@ -2,7 +2,7 @@ using Microsoft.AspNet.Mvc; using MvcSample.Models; -namespace MvcSample +namespace MvcSample.RandomNameSpace { public class Home2Controller { diff --git a/samples/MvcSample/HomeController.cs b/samples/MvcSample/HomeController.cs index db8481ef11..96ca422e06 100644 --- a/samples/MvcSample/HomeController.cs +++ b/samples/MvcSample/HomeController.cs @@ -1,5 +1,8 @@ using Microsoft.AspNet.Mvc; +<<<<<<< HEAD using MvcSample.Models; +======= +>>>>>>> Support per process caching of controller discovery namespace MvcSample { diff --git a/samples/MvcSample/Startup.cs b/samples/MvcSample/Startup.cs index 190086249e..341bdda3cd 100644 --- a/samples/MvcSample/Startup.cs +++ b/samples/MvcSample/Startup.cs @@ -2,6 +2,7 @@ using System; using System.IO; using Microsoft.AspNet.Abstractions; +using Microsoft.AspNet.DependencyInjection; using Microsoft.AspNet.Mvc; using Microsoft.AspNet.Mvc.Routing; using Microsoft.AspNet.Mvc.Startup; @@ -24,8 +25,11 @@ namespace MvcSample // HACK appbase doesn't seem to work. When in VS we're pointing at bin\Debug\Net45, so move up 3 directories string appRoot = Path.GetFullPath(Path.Combine(Environment.CurrentDirectory, "..", "..", "..")); - var serviceProvider = MvcServices.Create(appRoot); - var handler = new MvcHandler(serviceProvider); + var mvcServices = new MvcServices(appRoot); + + mvcServices.Finalize(); + + var handler = (MvcHandler)(ActivatorUtilities.CreateInstance(mvcServices.Services, typeof(MvcHandler))); builder.Run(async context => { diff --git a/src/Microsoft.AspNet.Mvc.Startup/MvcHandler.cs b/src/Microsoft.AspNet.Mvc.Startup/MvcHandler.cs index a42b534459..532767e517 100644 --- a/src/Microsoft.AspNet.Mvc.Startup/MvcHandler.cs +++ b/src/Microsoft.AspNet.Mvc.Startup/MvcHandler.cs @@ -8,20 +8,18 @@ namespace Microsoft.AspNet.Mvc { public class MvcHandler { - private readonly IServiceProvider _serviceProvider; + private readonly IActionInvokerFactory _actionInvokerFactory; - public MvcHandler(IServiceProvider serviceProvider) + public MvcHandler(IActionInvokerFactory actionInvokerFactory) { - _serviceProvider = serviceProvider; + _actionInvokerFactory = actionInvokerFactory; } public Task ExecuteAsync(HttpContext context, IRouteData routeData) { var requestContext = new RequestContext(context, routeData); - IActionInvokerFactory invokerFactory = _serviceProvider.GetService(); - - var invoker = invokerFactory.CreateInvoker(requestContext); + var invoker = _actionInvokerFactory.CreateInvoker(requestContext); return invoker.InvokeActionAsync(); } diff --git a/src/Microsoft.AspNet.Mvc.Startup/MvcServices.cs b/src/Microsoft.AspNet.Mvc.Startup/MvcServices.cs index 94cf7f7902..5d6d717ea8 100644 --- a/src/Microsoft.AspNet.Mvc.Startup/MvcServices.cs +++ b/src/Microsoft.AspNet.Mvc.Startup/MvcServices.cs @@ -1,31 +1,113 @@ -using Microsoft.AspNet.DependencyInjection; +using System; +using System.Collections; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using Microsoft.AspNet.DependencyInjection; using Microsoft.AspNet.FileSystems; using Microsoft.AspNet.Mvc.Razor; namespace Microsoft.AspNet.Mvc.Startup { - public static class MvcServices + public class MvcServices { - public static ServiceProvider Create(string appRoot) + private object _lock = new object(); + + private List _typesToFinalize = new List(); + + public ServiceProvider Services { get; private set; } + + public MvcServices(string appRoot) { - var services = new ServiceProvider(); - services.Add(); - services.Add(); - services.Add(); - services.Add(); - services.Add(); - services.Add(); + Services = new ServiceProvider(); - services.AddInstance(new PhysicalFileSystem(appRoot)); - services.AddInstance(new MvcRazorHost("Microsoft.AspNet.Mvc.Razor.RazorView")); - #if NET45 - services.Add(); - #endif - services.Add(); - services.Add(); - services.Add(); + AddAndRegisterForFinalization(); + AddAndRegisterForFinalization(); + AddAndRegisterForFinalization(); + AddAndRegisterForFinalization(); + AddAndRegisterForFinalization(); + AddAndRegisterForFinalization(); + AddAndRegisterForFinalization(); - return services; + // need singleton support here. + // AddAndRegisterForFinalization(); + AddInstanceAndRegisterForFinalization(new DefaultControllerCache(new DefaultSkipAssemblies())); + AddInstanceAndRegisterForFinalization(new PhysicalFileSystem(appRoot)); + AddInstanceAndRegisterForFinalization(new MvcRazorHost("Microsoft.AspNet.Mvc.Razor.RazorView")); + +#if NET45 + AddAndRegisterForFinalization(); +#endif + AddAndRegisterForFinalization(); + AddAndRegisterForFinalization(); + AddAndRegisterForFinalization(); + } + + public void AddAndRegisterForFinalization() where U : T + { + Services.Add(); +#if NET45 + if (typeof(IFinalizeSetup).IsAssignableFrom(typeof(U))) +#else + if (typeof(IFinalizeSetup).GetTypeInfo().IsAssignableFrom(typeof(U).GetTypeInfo())) +#endif + { + _typesToFinalize.Add(typeof(T)); + } + } + + public void AddInstanceAndRegisterForFinalization(object instance) + { + Services.AddInstance(instance); + + if ((instance as IFinalizeSetup) != null) + { + _typesToFinalize.Add(typeof(T)); + } + } + + public void Finalize() + { + if (_typesToFinalize == null) + { + return; + } + + // We want to lock around here so finalization happens just once. + // This is not a code intended to be used during request, so the lock is just a safety precaution. + lock (_lock) + { + if (_typesToFinalize == null) + { + return; + } + + foreach (var markerType in _typesToFinalize) + { + var services = this.Services.GetService(markerType); + + var serviceToFinalize = services as IFinalizeSetup; + + if (serviceToFinalize != null) + { + serviceToFinalize.FinalizeSetup(); + } + else + { + var setOfServices = services as IEnumerable; + + if (setOfServices != null) + { + foreach (var service in setOfServices.OfType()) + { + service.FinalizeSetup(); + } + } + } + } + + _typesToFinalize = null; + } } } } diff --git a/src/Microsoft.AspNet.Mvc/ActionInvokerProvider.cs b/src/Microsoft.AspNet.Mvc/ActionInvokerProvider.cs index bdc6e6b2f1..0707b1afa9 100644 --- a/src/Microsoft.AspNet.Mvc/ActionInvokerProvider.cs +++ b/src/Microsoft.AspNet.Mvc/ActionInvokerProvider.cs @@ -1,16 +1,20 @@ using System; +using Microsoft.AspNet.DependencyInjection; namespace Microsoft.AspNet.Mvc { public class ActionInvokerProvider : IActionInvokerProvider { - private IActionResultFactory _actionResultFactory; - private IServiceProvider _serviceProvider; + private readonly IActionResultFactory _actionResultFactory; + private readonly IServiceProvider _serviceProvider; + private readonly IControllerFactory _controrllerFactory; public ActionInvokerProvider(IActionResultFactory actionResultFactory, + IControllerFactory controllerFactory, IServiceProvider serviceProvider) { _actionResultFactory = actionResultFactory; + _controrllerFactory = controllerFactory; _serviceProvider = serviceProvider; } @@ -24,6 +28,7 @@ namespace Microsoft.AspNet.Mvc requestContext, controllerActionDescriptor, _actionResultFactory, + _controrllerFactory, _serviceProvider); } diff --git a/src/Microsoft.AspNet.Mvc/ControllerActionInvoker.cs b/src/Microsoft.AspNet.Mvc/ControllerActionInvoker.cs index c5c7dcc599..c6e9bcff79 100644 --- a/src/Microsoft.AspNet.Mvc/ControllerActionInvoker.cs +++ b/src/Microsoft.AspNet.Mvc/ControllerActionInvoker.cs @@ -14,22 +14,24 @@ namespace Microsoft.AspNet.Mvc private readonly ControllerBasedActionDescriptor _descriptor; private readonly IActionResultFactory _actionResultFactory; private readonly IServiceProvider _serviceProvider; + private readonly IControllerFactory _controllerFactory; public ControllerActionInvoker(RequestContext requestContext, ControllerBasedActionDescriptor descriptor, IActionResultFactory actionResultFactory, + IControllerFactory controllerFactory, IServiceProvider serviceProvider) { _requestContext = requestContext; _descriptor = descriptor; _actionResultFactory = actionResultFactory; + _controllerFactory = controllerFactory; _serviceProvider = serviceProvider; } public Task InvokeActionAsync() { - var factory = _serviceProvider.GetService(); - object controller = factory.CreateController(_requestContext.HttpContext, _descriptor.ControllerName); + object controller = _controllerFactory.CreateController(_requestContext.HttpContext, _descriptor.ControllerName); if (controller == null) { diff --git a/src/Microsoft.AspNet.Mvc/ControllerCache.cs b/src/Microsoft.AspNet.Mvc/ControllerCache.cs new file mode 100644 index 0000000000..1d6267a64e --- /dev/null +++ b/src/Microsoft.AspNet.Mvc/ControllerCache.cs @@ -0,0 +1,9 @@ +using System.Collections.Generic; + +namespace Microsoft.AspNet.Mvc +{ + public abstract class ControllerCache + { + public abstract IEnumerable GetController(string controllerName); + } +} diff --git a/src/Microsoft.AspNet.Mvc/ControllerDescriptor.cs b/src/Microsoft.AspNet.Mvc/ControllerDescriptor.cs new file mode 100644 index 0000000000..aaaa208ac1 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc/ControllerDescriptor.cs @@ -0,0 +1,35 @@ +using System; +using System.Reflection; + +namespace Microsoft.AspNet.Mvc +{ + public class ControllerDescriptor + { + public ControllerDescriptor(Type controllerType, Assembly assembly) + { + if (controllerType == null) + { + throw new ArgumentNullException("controllerType"); + } + + if (assembly == null) + { + throw new ArgumentNullException("assembly"); + } + + ControllerType = controllerType; + Assembly = assembly; + + ControllerName = controllerType.Name; + AssemblyName = assembly.GetName().Name; + } + + public string ControllerName { get; private set; } + + public string AssemblyName { get; private set; } + + public Type ControllerType { get; private set; } + + public Assembly Assembly { get; private set; } + } +} diff --git a/src/Microsoft.AspNet.Mvc/DefaultControllerCache.cs b/src/Microsoft.AspNet.Mvc/DefaultControllerCache.cs new file mode 100644 index 0000000000..3a54073b51 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc/DefaultControllerCache.cs @@ -0,0 +1,79 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; + +namespace Microsoft.AspNet.Mvc +{ + public class DefaultControllerCache : ControllerCache, IFinalizeSetup + { + private readonly SkipAssemblies _skipAssemblies; + + public IReadOnlyDictionary> Controllers { get; protected set; } + + public virtual void FinalizeSetup() + { + Controllers = ScanAppDomain(); + } + + public DefaultControllerCache(SkipAssemblies skipAssemblies) + { + _skipAssemblies = skipAssemblies ?? new SkipNoAssemblies(); + } + + public override IEnumerable GetController(string controllerName) + { + if (Controllers == null) + { + throw new InvalidOperationException("Finalizing the setup must happen prior to accessing controllers"); + } + + return Controllers[controllerName]; + } + + public Dictionary> ScanAppDomain() + { + var dictionary = new Dictionary>(StringComparer.Ordinal); + + foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies().Where(AllowAssembly)) + { + foreach (var type in assembly.DefinedTypes.Where(IsController).Select(info => info.AsType())) + { + var descriptor = new ControllerDescriptor(type, assembly); + + IEnumerable controllerDescriptors; + if (!dictionary.TryGetValue(type.Name, out controllerDescriptors)) + { + controllerDescriptors = new List(); + dictionary.Add(descriptor.ControllerName, controllerDescriptors); + } + + ((List)controllerDescriptors).Add(descriptor); + } + } + + return dictionary; + } + + public virtual bool IsController(TypeInfo typeInfo) + { + if (typeInfo == null) + { + throw new ArgumentNullException("typeInfo"); + } + + bool validController = typeInfo.IsClass && + !typeInfo.IsAbstract && + !typeInfo.ContainsGenericParameters; + + validController = validController && typeInfo.Name.EndsWith("Controller", StringComparison.OrdinalIgnoreCase); + + return validController; + } + + private bool AllowAssembly(Assembly assembly) + { + return !_skipAssemblies.Skip(assembly, SkipAssemblies.ControllerDiscoveryScope); + } + } +} diff --git a/src/Microsoft.AspNet.Mvc/DefaultControllerFactory.cs b/src/Microsoft.AspNet.Mvc/DefaultControllerFactory.cs index 35e9d7c0c2..38782c76a7 100644 --- a/src/Microsoft.AspNet.Mvc/DefaultControllerFactory.cs +++ b/src/Microsoft.AspNet.Mvc/DefaultControllerFactory.cs @@ -1,5 +1,4 @@ using System; -using System.Diagnostics; using System.Linq; using System.Reflection; using Microsoft.AspNet.Abstractions; @@ -10,10 +9,12 @@ namespace Microsoft.AspNet.Mvc public class DefaultControllerFactory : IControllerFactory { private readonly IServiceProvider _serviceProvider; + private readonly ControllerCache _controllerCache; - public DefaultControllerFactory(IServiceProvider serviceProvider) + public DefaultControllerFactory(IServiceProvider serviceProvider, ControllerCache cache) { _serviceProvider = serviceProvider; + _controllerCache = cache; } public object CreateController(HttpContext context, string controllerName) @@ -23,29 +24,26 @@ namespace Microsoft.AspNet.Mvc controllerName += "Controller"; } - foreach (var a in AppDomain.CurrentDomain.GetAssemblies()) - { - try - { - var type = a.GetType(controllerName) ?? - a.GetType(a.GetName().Name + "." + controllerName); -#if NET45 - type = type ?? a.GetTypes().FirstOrDefault(t => t.Name.Equals(controllerName, StringComparison.OrdinalIgnoreCase)); -#endif + var controllers = _controllerCache.GetController(controllerName); - if (type != null) + try + { + var type = controllers.SingleOrDefault().ControllerType; + + if (type != null) + { + try { return ActivatorUtilities.CreateInstance(_serviceProvider, type); } + catch (ReflectionTypeLoadException) + { + } } - catch (ReflectionTypeLoadException) - { - // TODO: Trace here - } - catch (Exception) - { - // TODO: Trace here - } + } + catch (InvalidOperationException) + { + throw new InvalidOperationException("Ambiguity: Duplicate controllers match the controller name"); } return null; diff --git a/src/Microsoft.AspNet.Mvc/DefaultSkipAssemblies.cs b/src/Microsoft.AspNet.Mvc/DefaultSkipAssemblies.cs new file mode 100644 index 0000000000..65c31ff05f --- /dev/null +++ b/src/Microsoft.AspNet.Mvc/DefaultSkipAssemblies.cs @@ -0,0 +1,141 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Reflection; + +namespace Microsoft.AspNet.Mvc +{ + public class DefaultSkipAssemblies : SkipAssemblies + { + private HashSet _hash = new HashSet(StringComparer.OrdinalIgnoreCase); + + public DefaultSkipAssemblies(IEnumerable assemblyNames) + { + InitializeHash(assemblyNames); + } + + public DefaultSkipAssemblies() + { +#if NET45 + InitializeHash(@" +klr.net45.managed +Microsoft.Net.Runtime.Interfaces +klr.host +System +System.Core +System.Configuration +System.Xml +Microsoft.Net.ApplicationHost +Microsoft.Net.Runtime +Newtonsoft.Json +System.Numerics +System.ComponentModel.DataAnnotations +System.Runtime.Serialization +System.Xml.Linq +System.Data +Microsoft.CodeAnalysis +System.Collections.Immutable +System.Runtime +Microsoft.CodeAnalysis.CSharp +System.IO.Compression +Microsoft.AspNet.FileSystems +Microsoft.AspNet.Abstractions +Microsoft.AspNet.DependencyInjection +Microsoft.AspNet.Razor +Newtonsoft.Json +System.Linq +System.Collections +System.Runtime.Extensions +System.Threading +System.Reflection.Metadata.Ecma335 +Microsoft.AspNet.Mvc.ModelBinding +Microsoft.AspNet.Mvc.Rendering +Microsoft.AspNet.Mvc +Microsoft.AspNet.Mvc.Razor.Host +Microsoft.AspNet.Mvc.Razor +Microsoft.AspNet.Mvc.Startup +Owin +Microsoft.Owin +Microsoft.Owin.Diagnostics +Microsoft.Owin.Hosting +Microsoft.Owin.Host.HttpListener +Microsoft.AspNet.AppBuilderSupport +Anonymously Hosted DynamicMethods Assembly +Microsoft.AspNet.PipelineCore +Microsoft.AspNet.FeatureModel +mscorlib +klr.net45.managed +Microsoft.Net.Runtime.Interfaces +klr.host +System +System.Core +System.Configuration +System.Xml +Microsoft.Net.ApplicationHost +Microsoft.Net.Runtime +Newtonsoft.Json +System.Numerics +System.ComponentModel.DataAnnotations +System.Runtime.Serialization +System.Xml.Linq +System.Data +Microsoft.CodeAnalysis +System.Collections.Immutable +System.Runtime +Microsoft.CodeAnalysis.CSharp +System.IO.Compression +Microsoft.AspNet.FileSystems +Microsoft.AspNet.Abstractions +Microsoft.AspNet.DependencyInjection +Microsoft.AspNet.Razor +Newtonsoft.Json +System.Linq +System.Collections +System.Runtime.Extensions +System.Threading +System.Reflection.Metadata.Ecma335 +Microsoft.AspNet.Mvc.ModelBinding +Microsoft.AspNet.Mvc.Rendering +Microsoft.AspNet.Mvc +Microsoft.AspNet.Mvc.Razor.Host +Microsoft.AspNet.Mvc.Razor +Microsoft.AspNet.Mvc.Startup +Owin +Microsoft.Owin +Microsoft.Owin.Diag".Split(new char[] { '\r', '\n'}, StringSplitOptions.RemoveEmptyEntries)); +#else +#endif + } + + private void InitializeHash(IEnumerable assemblyNames) + { + if (assemblyNames == null) + { + throw new ArgumentNullException("assemblyNames"); + } + + foreach (var assemblyName in assemblyNames) + { + if (!string.IsNullOrWhiteSpace(assemblyName)) + { + _hash.Add(assemblyName); + } + } + } + + public override bool Skip(Assembly assembly, string scope) + { + if (scope == null || + !string.Equals(scope, SkipAssemblies.ControllerDiscoveryScope, StringComparison.Ordinal)) + { + return false; + } + + string name = assembly.GetName().Name; + + bool contains = _hash.Contains(name); + + return contains; + } + } +} diff --git a/src/Microsoft.AspNet.Mvc/IFinalizeSetup.cs b/src/Microsoft.AspNet.Mvc/IFinalizeSetup.cs new file mode 100644 index 0000000000..d67e78f8df --- /dev/null +++ b/src/Microsoft.AspNet.Mvc/IFinalizeSetup.cs @@ -0,0 +1,7 @@ +namespace Microsoft.AspNet.Mvc +{ + public interface IFinalizeSetup + { + void FinalizeSetup(); + } +} diff --git a/src/Microsoft.AspNet.Mvc/SkipAssemblies.cs b/src/Microsoft.AspNet.Mvc/SkipAssemblies.cs new file mode 100644 index 0000000000..7bd2b7d27f --- /dev/null +++ b/src/Microsoft.AspNet.Mvc/SkipAssemblies.cs @@ -0,0 +1,11 @@ +using System.Reflection; + +namespace Microsoft.AspNet.Mvc +{ + public abstract class SkipAssemblies + { + public static readonly string ControllerDiscoveryScope = "DCS"; + + public abstract bool Skip(Assembly assembly, string scope); + } +} diff --git a/src/Microsoft.AspNet.Mvc/SkipNoAssemblies.cs b/src/Microsoft.AspNet.Mvc/SkipNoAssemblies.cs new file mode 100644 index 0000000000..bc30222f1e --- /dev/null +++ b/src/Microsoft.AspNet.Mvc/SkipNoAssemblies.cs @@ -0,0 +1,12 @@ +using System.Reflection; + +namespace Microsoft.AspNet.Mvc +{ + public class SkipNoAssemblies : SkipAssemblies + { + public override bool Skip(Assembly assembly, string scope) + { + return false; + } + } +}