From 0d603a38cf30db3c7100f26897561111588bee73 Mon Sep 17 00:00:00 2001 From: YishaiGalatzer Date: Sun, 12 Oct 2014 16:37:17 -0700 Subject: [PATCH] PR feedback and sort/clean MvcServices --- .../Compilation/CompilerCache.cs | 9 ++ .../Compilation/ICompilerCache.cs | 12 +- .../Razor/IRazorCompilationService.cs | 3 +- .../Razor/RazorCompilationService.cs | 2 - .../VirtualPathRazorPageFactory.cs | 5 +- src/Microsoft.AspNet.Mvc/MvcServices.cs | 124 +++++++++++------- 6 files changed, 98 insertions(+), 57 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs index 298d45c7b5..4618542916 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs @@ -9,11 +9,19 @@ using System.Reflection; namespace Microsoft.AspNet.Mvc.Razor { + /// public class CompilerCache : ICompilerCache { private readonly ConcurrentDictionary _cache; private static readonly Type[] EmptyType = new Type[0]; + /// + /// Sets up the runtime compilation cache. + /// + /// + /// An representing the assemblies + /// used to search for pre-compiled views. + /// public CompilerCache([NotNull] IAssemblyProvider provider) : this(GetFileInfos(provider.CandidateAssemblies)) { @@ -64,6 +72,7 @@ namespace Microsoft.AspNet.Mvc.Razor return false; } + /// public CompilationResult GetOrAdd([NotNull] RelativeFileInfo fileInfo, bool enableInstrumentation, [NotNull] Func compile) diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs index d11d4a4a30..3d06c9ab89 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs @@ -2,12 +2,22 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; -using Microsoft.AspNet.FileSystems; namespace Microsoft.AspNet.Mvc.Razor { + /// + /// Caches the result of runtime compilation for the duration of the app lifetime. + /// public interface ICompilerCache { + /// + /// Get an existing compilation result, or create and add a new one if it is + /// not available in the cache. + /// + /// A representing the file. + /// to generate instrumentation. + /// An delegate that will generate a compilation result. + /// A cached . CompilationResult GetOrAdd([NotNull] RelativeFileInfo fileInfo, bool enableInstrumentation, [NotNull] Func compile); diff --git a/src/Microsoft.AspNet.Mvc.Razor/Razor/IRazorCompilationService.cs b/src/Microsoft.AspNet.Mvc.Razor/Razor/IRazorCompilationService.cs index d9a59c2443..a56e9eb146 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Razor/IRazorCompilationService.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Razor/IRazorCompilationService.cs @@ -14,7 +14,8 @@ namespace Microsoft.AspNet.Mvc.Razor /// A instance that represents the file to compile. /// /// Indicates that the page should be instrumented. - /// A that represents the results of parsing and compiling the file. + /// + /// A that represents the results of parsing and compiling the file. /// CompilationResult Compile(RelativeFileInfo fileInfo, bool isInstrumented); } diff --git a/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs b/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs index 2258be0038..a6a34eb1f2 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs @@ -1,10 +1,8 @@ // Copyright (c) Microsoft Open Technologies, Inc. All rights reserved. // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. -using System; using System.Linq; using Microsoft.AspNet.Razor; -using Microsoft.Framework.DependencyInjection; namespace Microsoft.AspNet.Mvc.Razor { diff --git a/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs b/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs index 3fd41f329a..eb1addcbd9 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs @@ -40,8 +40,9 @@ namespace Microsoft.AspNet.Mvc.Razor { if (_razorcompilationService == null) { - // it is ok to use the cached service provider because this service has - // a lifetime of Scoped. + // it is ok to use the cached service provider because both this, and the + // resolved service are in a lifetime of Scoped. + // We don't want to get it upgront because it will force Roslyn to load. _razorcompilationService = _serviceProvider.GetService(); } diff --git a/src/Microsoft.AspNet.Mvc/MvcServices.cs b/src/Microsoft.AspNet.Mvc/MvcServices.cs index 9154842654..d14339ab96 100644 --- a/src/Microsoft.AspNet.Mvc/MvcServices.cs +++ b/src/Microsoft.AspNet.Mvc/MvcServices.cs @@ -31,58 +31,47 @@ namespace Microsoft.AspNet.Mvc { var describe = new ServiceDescriber(configuration); + // + // Options and core services. + // yield return describe.Transient, MvcOptionsSetup>(); + yield return describe.Transient(); + yield return describe.Transient(typeof(INestedProviderManager<>), typeof(NestedProviderManager<>)); + yield return describe.Transient(typeof(INestedProviderManagerAsync<>), typeof(NestedProviderManagerAsync<>)); + yield return describe.Transient(); + // + // Core action discovery, filters and action execution. + // + yield return describe.Transient(); yield return describe.Transient(); yield return describe.Singleton(); - - yield return describe.Singleton(); - yield return describe.Scoped(); + yield return describe.Transient(); // This provider needs access to the per-request services, but might be used many times for a given // request. yield return describe.Scoped, DefaultActionConstraintProvider>(); - yield return describe.Transient(); - yield return describe.Transient(); - yield return describe.Transient(); - - // The host is designed to be discarded after consumption and is very inexpensive to initialize. - yield return describe.Transient(); - - yield return describe.Singleton(); - yield return describe.Singleton(); - yield return describe.Singleton(); - - // The provider is inexpensive to initialize and provides ViewEngines that may require request - // specific services. - yield return describe.Transient(); - yield return describe.Scoped(); - // The ViewStartProvider needs to be able to consume scoped instances of IRazorPageFactory - yield return describe.Scoped(); - yield return describe.Transient(); - - // Transient since the IViewLocationExpanders returned by the instance is cached by view engines. - yield return describe.Transient(); - // Caches view locations that are valid for the lifetime of the application. - yield return describe.Singleton(); - - // Only want one ITagHelperActivator so it can cache Type activation information. Types won't conflict. - yield return describe.Singleton(); - - yield return describe.Singleton(); - // Virtual path view factory needs to stay scoped so views can get get scoped services. - yield return describe.Scoped(); - yield return describe.Singleton(); + yield return describe.Singleton(); + yield return describe.Scoped(); yield return describe.Transient, - ControllerActionDescriptorProvider>(); + ControllerActionDescriptorProvider>(); yield return describe.Transient, ControllerActionInvokerProvider>(); yield return describe.Singleton(); + // The IGlobalFilterProvider is used to build the action descriptors (likely once) and so should + // remain transient to avoid keeping it in memory. + yield return describe.Transient(); + + yield return describe.Transient, DefaultFilterProvider>(); + + // + // Dataflow - ModelBinding, Validation and Formatting + // yield return describe.Transient(); yield return describe.Scoped(); @@ -98,17 +87,53 @@ namespace Microsoft.AspNet.Mvc yield return describe.Instance( new JsonOutputFormatter(JsonOutputFormatter.CreateDefaultSettings(), indent: false)); - // The IGlobalFilterProvider is used to build the action descriptors (likely once) and so should - // remain transient to avoid keeping it in memory. - yield return describe.Transient(); - - yield return describe.Transient, DefaultFilterProvider>(); - yield return describe.Transient(); yield return describe.Scoped(); + yield return describe.Transient(); + // + // Razor, Views and runtime compilation + // + + // The provider is inexpensive to initialize and provides ViewEngines that may require request + // specific services. + yield return describe.Scoped(); + yield return describe.Transient(); + // Transient since the IViewLocationExpanders returned by the instance is cached by view engines. + yield return describe.Transient(); + // Caches view locations that are valid for the lifetime of the application. + yield return describe.Singleton(); + yield return describe.Singleton(); + + // The host is designed to be discarded after consumption and is very inexpensive to initialize. + yield return describe.Transient(); + + yield return describe.Singleton(); + yield return describe.Singleton(); + + // Both the compiler cache and roslyn compilation service hold on the compilation related + // caches. RazorCompilation service is just an adapter service, and it is scoped + // since it will typically be resolved multiple times per request. + yield return describe.Scoped(); + + // The ViewStartProvider needs to be able to consume scoped instances of IRazorPageFactory + yield return describe.Scoped(); + yield return describe.Transient(); + yield return describe.Singleton(); + // Virtual path view factory needs to stay scoped so views can get get scoped services. + yield return describe.Scoped(); + + // + // View and rendering helpers + // + + yield return describe.Transient(); + yield return describe.Transient(typeof(IHtmlHelper<>), typeof(HtmlHelper<>)); yield return describe.Scoped(); + // Only want one ITagHelperActivator so it can cache Type activation information. Types won't conflict. + yield return describe.Singleton(); + yield return describe.Transient(); yield return describe.Singleton(); yield return describe.Transient(); @@ -116,7 +141,9 @@ namespace Microsoft.AspNet.Mvc DefaultViewComponentInvokerProvider>(); yield return describe.Transient(); - yield return describe.Transient(); + // + // Security and Authorization + // yield return describe.Transient(); yield return describe.Singleton(); @@ -124,19 +151,14 @@ namespace Microsoft.AspNet.Mvc yield return describe.Singleton(); + // + // Api Description + // + yield return describe.Singleton(); yield return describe.Transient, DefaultApiDescriptionProvider>(); - - yield return describe.Transient(typeof(INestedProviderManager<>), typeof(NestedProviderManager<>)); - - yield return describe.Transient(typeof(INestedProviderManagerAsync<>), typeof(NestedProviderManagerAsync<>)); - - yield return describe.Transient(); - yield return describe.Transient(typeof(IHtmlHelper<>), typeof(HtmlHelper<>)); - - yield return describe.Transient(); } } }