PR feedback and sort/clean MvcServices

src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs

@@ -9,11 +9,19 @@ using System.Reflection;
 
 namespace Microsoft.AspNet.Mvc.Razor
 {
+    /// <inheritdoc />
     public class CompilerCache : ICompilerCache
     {
         private readonly ConcurrentDictionary<string, CompilerCacheEntry> _cache;
         private static readonly Type[] EmptyType = new Type[0];
 
+        /// <summary>
+        /// Sets up the runtime compilation cache.
+        /// </summary>
+        /// <param name="provider">
+        /// An <see cref="IAssemblyProvider"/> representing the assemblies
+        /// used to search for pre-compiled views.
+        /// </param>
         public CompilerCache([NotNull] IAssemblyProvider provider)
             : this(GetFileInfos(provider.CandidateAssemblies))
         {
@@ -64,6 +72,7 @@ namespace Microsoft.AspNet.Mvc.Razor
             return false;
         }
 
+        /// <inheritdoc />
         public CompilationResult GetOrAdd([NotNull] RelativeFileInfo fileInfo,
                                           bool enableInstrumentation,
                                           [NotNull] Func<CompilationResult> compile)

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
 {
+    /// <summary>
+    /// Caches the result of runtime compilation for the duration of the app lifetime.
+    /// </summary>
     public interface ICompilerCache
     {
+        /// <summary>
+        /// Get an existing compilation result, or create and add a new one if it is
+        /// not available in the cache.
+        /// </summary>
+        /// <param name="fileInfo">A <see cref="RelativeFileInfo"/> representing the file.</param>
+        /// <param name="enableInstrumentation"><see langword="true"/> to generate instrumentation.</param>
+        /// <param name="compile">An delegate that will generate a compilation result.</param>
+        /// <returns>A cached <see cref="CompilationResult"/>.</returns>
         CompilationResult GetOrAdd([NotNull] RelativeFileInfo fileInfo,
                                    bool enableInstrumentation,
                                    [NotNull] Func<CompilationResult> compile);

src/Microsoft.AspNet.Mvc.Razor/Razor/IRazorCompilationService.cs

@@ -14,7 +14,8 @@ namespace Microsoft.AspNet.Mvc.Razor
         /// <param name="fileInfo">A <see cref="RelativeFileInfo"/> instance that represents the file to compile.
         /// </param>
         /// <param name="isInstrumented">Indicates that the page should be instrumented.</param>
-        /// <returns>A <see cref="CompilationResult"/> that represents the results of parsing and compiling the file.
+        /// <returns>
+        /// A <see cref="CompilationResult"/> that represents the results of parsing and compiling the file.
         /// </returns>
         CompilationResult Compile(RelativeFileInfo fileInfo, bool isInstrumented);
     }

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
 {

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<IRazorCompilationService>();
                 }
 

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<IOptionsAction<MvcOptions>, MvcOptionsSetup>();
+            yield return describe.Transient<IAssemblyProvider, DefaultAssemblyProvider>();
+            yield return describe.Transient(typeof(INestedProviderManager<>), typeof(NestedProviderManager<>));
+            yield return describe.Transient(typeof(INestedProviderManagerAsync<>), typeof(NestedProviderManagerAsync<>));
+            yield return describe.Transient<MvcMarkerService, MvcMarkerService>();
 
+            //
+            // Core action discovery, filters and action execution.
+            //
+            yield return describe.Transient<IActionDiscoveryConventions, DefaultActionDiscoveryConventions>();
             yield return describe.Transient<IControllerFactory, DefaultControllerFactory>();
             yield return describe.Singleton<IControllerActivator, DefaultControllerActivator>();
-
-            yield return describe.Singleton<IActionSelectorDecisionTreeProvider, ActionSelectorDecisionTreeProvider>();
-            yield return describe.Scoped<IActionSelector, DefaultActionSelector>();
+            yield return describe.Transient<IActionInvokerFactory, ActionInvokerFactory>();
 
             // This provider needs access to the per-request services, but might be used many times for a given
             // request.
             yield return describe.Scoped<INestedProvider<ActionConstraintProviderContext>,
                 DefaultActionConstraintProvider>();
 
-            yield return describe.Transient<IActionInvokerFactory, ActionInvokerFactory>();
-            yield return describe.Transient<IAssemblyProvider, DefaultAssemblyProvider>();
-            yield return describe.Transient<IActionDiscoveryConventions, DefaultActionDiscoveryConventions>();
-
-            // The host is designed to be discarded after consumption and is very inexpensive to initialize.
-            yield return describe.Transient<IMvcRazorHost, MvcRazorHost>();
-
-            yield return describe.Singleton<ICompilerCache, CompilerCache>();
-            yield return describe.Singleton<ICompilationService, RoslynCompilationService>();
-            yield return describe.Singleton<IRazorCompilationService, RazorCompilationService>();
-
-            // The provider is inexpensive to initialize and provides ViewEngines that may require request
-            // specific services.
-            yield return describe.Transient<IViewEngineProvider, DefaultViewEngineProvider>();
-            yield return describe.Scoped<ICompositeViewEngine, CompositeViewEngine>();
-            // The ViewStartProvider needs to be able to consume scoped instances of IRazorPageFactory
-            yield return describe.Scoped<IViewStartProvider, ViewStartProvider>();
-            yield return describe.Transient<IRazorView, RazorView>();
-
-            // Transient since the IViewLocationExpanders returned by the instance is cached by view engines.
-            yield return describe.Transient<IViewLocationExpanderProvider, DefaultViewLocationExpanderProvider>();
-            // Caches view locations that are valid for the lifetime of the application.
-            yield return describe.Singleton<IViewLocationCache, DefaultViewLocationCache>();
-
-            // Only want one ITagHelperActivator so it can cache Type activation information. Types won't conflict.
-            yield return describe.Singleton<ITagHelperActivator, DefaultTagHelperActivator>();
-
-            yield return describe.Singleton<IRazorPageActivator, RazorPageActivator>();
-            // Virtual path view factory needs to stay scoped so views can get get scoped services.
-            yield return describe.Scoped<IRazorPageFactory, VirtualPathRazorPageFactory>();
-            yield return describe.Singleton<IFileInfoCache, ExpiringFileInfoCache>();
+            yield return describe.Singleton<IActionSelectorDecisionTreeProvider, ActionSelectorDecisionTreeProvider>();
+            yield return describe.Scoped<IActionSelector, DefaultActionSelector>();
 
             yield return describe.Transient<INestedProvider<ActionDescriptorProviderContext>,
-                                            ControllerActionDescriptorProvider>();
+                                ControllerActionDescriptorProvider>();
             yield return describe.Transient<INestedProvider<ActionInvokerProviderContext>,
                                             ControllerActionInvokerProvider>();
             yield return describe.Singleton<IActionDescriptorsCollectionProvider,
                 DefaultActionDescriptorsCollectionProvider>();
 
+            // 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<IGlobalFilterProvider, DefaultGlobalFilterProvider>();
+
+            yield return describe.Transient<INestedProvider<FilterProviderContext>, DefaultFilterProvider>();
+
+            //
+            // Dataflow - ModelBinding, Validation and Formatting
+            //
             yield return describe.Transient<IModelMetadataProvider, DataAnnotationsModelMetadataProvider>();
             yield return describe.Scoped<IActionBindingContextProvider, DefaultActionBindingContextProvider>();
 
@@ -98,17 +87,53 @@ namespace Microsoft.AspNet.Mvc
             yield return describe.Instance<JsonOutputFormatter>(
                 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<IGlobalFilterProvider, DefaultGlobalFilterProvider>();
-
-            yield return describe.Transient<INestedProvider<FilterProviderContext>, DefaultFilterProvider>();
-
             yield return describe.Transient<IModelValidatorProviderProvider, DefaultModelValidatorProviderProvider>();
             yield return describe.Scoped<ICompositeModelValidatorProvider, CompositeModelValidatorProvider>();
+            yield return describe.Transient<IBodyModelValidator, DefaultBodyModelValidator>();
 
+            //
+            // Razor, Views and runtime compilation
+            //
+
+            // The provider is inexpensive to initialize and provides ViewEngines that may require request
+            // specific services.
+            yield return describe.Scoped<ICompositeViewEngine, CompositeViewEngine>();
+            yield return describe.Transient<IViewEngineProvider, DefaultViewEngineProvider>();
+            // Transient since the IViewLocationExpanders returned by the instance is cached by view engines.
+            yield return describe.Transient<IViewLocationExpanderProvider, DefaultViewLocationExpanderProvider>();
+            // Caches view locations that are valid for the lifetime of the application.
+            yield return describe.Singleton<IViewLocationCache, DefaultViewLocationCache>();
+            yield return describe.Singleton<IFileInfoCache, ExpiringFileInfoCache>();
+
+            // The host is designed to be discarded after consumption and is very inexpensive to initialize.
+            yield return describe.Transient<IMvcRazorHost, MvcRazorHost>();
+
+            yield return describe.Singleton<ICompilerCache, CompilerCache>();
+            yield return describe.Singleton<ICompilationService, RoslynCompilationService>();
+
+            // 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<IRazorCompilationService, RazorCompilationService>();
+
+            // The ViewStartProvider needs to be able to consume scoped instances of IRazorPageFactory
+            yield return describe.Scoped<IViewStartProvider, ViewStartProvider>();
+            yield return describe.Transient<IRazorView, RazorView>();
+            yield return describe.Singleton<IRazorPageActivator, RazorPageActivator>();
+            // Virtual path view factory needs to stay scoped so views can get get scoped services.
+            yield return describe.Scoped<IRazorPageFactory, VirtualPathRazorPageFactory>();
+
+            //
+            // View and rendering helpers
+            //
+
+            yield return describe.Transient<IHtmlHelper, HtmlHelper>();
+            yield return describe.Transient(typeof(IHtmlHelper<>), typeof(HtmlHelper<>));
             yield return describe.Scoped<IUrlHelper, UrlHelper>();
 
+            // Only want one ITagHelperActivator so it can cache Type activation information. Types won't conflict.
+            yield return describe.Singleton<ITagHelperActivator, DefaultTagHelperActivator>();
+
             yield return describe.Transient<IViewComponentSelector, DefaultViewComponentSelector>();
             yield return describe.Singleton<IViewComponentActivator, DefaultViewComponentActivator>();
             yield return describe.Transient<IViewComponentInvokerFactory, DefaultViewComponentInvokerFactory>();
@@ -116,7 +141,9 @@ namespace Microsoft.AspNet.Mvc
                 DefaultViewComponentInvokerProvider>();
             yield return describe.Transient<IViewComponentHelper, DefaultViewComponentHelper>();
 
-            yield return describe.Transient<IBodyModelValidator, DefaultBodyModelValidator>();
+            //
+            // Security and Authorization
+            //
 
             yield return describe.Transient<IAuthorizationService, DefaultAuthorizationService>();
             yield return describe.Singleton<IClaimUidExtractor, DefaultClaimUidExtractor>();
@@ -124,19 +151,14 @@ namespace Microsoft.AspNet.Mvc
             yield return describe.Singleton<IAntiForgeryAdditionalDataProvider,
                 DefaultAntiForgeryAdditionalDataProvider>();
 
+            //
+            // Api Description
+            //
+
             yield return describe.Singleton<IApiDescriptionGroupCollectionProvider,
                 ApiDescriptionGroupCollectionProvider>();
             yield return describe.Transient<INestedProvider<ApiDescriptionProviderContext>,
                 DefaultApiDescriptionProvider>();
-
-            yield return describe.Transient(typeof(INestedProviderManager<>), typeof(NestedProviderManager<>));
-
-            yield return describe.Transient(typeof(INestedProviderManagerAsync<>), typeof(NestedProviderManagerAsync<>));
-
-            yield return describe.Transient<IHtmlHelper, HtmlHelper>();
-            yield return describe.Transient(typeof(IHtmlHelper<>), typeof(HtmlHelper<>));
-
-            yield return describe.Transient<MvcMarkerService, MvcMarkerService>();
         }
     }
 }