Changes per PR comments

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

@@ -10,83 +10,63 @@ namespace Microsoft.AspNet.Mvc.Razor.Compilation
     /// <summary>
     /// Represents the result of compilation.
     /// </summary>
-    public class CompilationResult
+    public struct CompilationResult
     {
         /// <summary>
-        /// Creates a new instance of <see cref="CompilationResult"/>.
+        /// Initializes a new instance of <see cref="CompilationResult"/> for a successful compilation.
         /// </summary>
-        protected CompilationResult()
+        /// <param name="type">The compiled type.</param>
+        public CompilationResult(Type type)
         {
+            if (type == null)
+            {
+                throw new ArgumentNullException(nameof(type));
+            }
+
+            CompiledType = type;
+            CompilationFailures = null;
         }
 
         /// <summary>
-        /// Gets (or sets in derived types) the type produced as a result of compilation.
+        /// Initializes a new instance of <see cref="CompilationResult"/> for a failed compilation.
         /// </summary>
-        /// <remarks>This property is <c>null</c> when compilation failed.</remarks>
-        public Type CompiledType { get; protected set; }
+        /// <param name="compilationFailures"><see cref="CompilationFailure"/>s produced from parsing or
+        /// compiling the Razor file.</param>
+        public CompilationResult(IEnumerable<CompilationFailure> compilationFailures)
+        {
+            if (compilationFailures == null)
+            {
+                throw new ArgumentNullException(nameof(compilationFailures));
+            }
+
+            CompiledType = null;
+            CompilationFailures = compilationFailures;
+        }
 
         /// <summary>
-        /// Gets (or sets in derived types) the generated C# content that was compiled.
+        /// Gets the type produced as a result of compilation.
         /// </summary>
-        public string CompiledContent { get; protected set; }
+        /// <remarks>This property is <c>null</c> when compilation failed.</remarks>
+        public Type CompiledType { get; }
 
         /// <summary>
         /// Gets the <see cref="CompilationFailure"/>s produced from parsing or compiling the Razor file.
         /// </summary>
         /// <remarks>This property is <c>null</c> when compilation succeeded. An empty sequence
         /// indicates a failed compilation.</remarks>
-        public IEnumerable<CompilationFailure> CompilationFailures { get; private set; }
+        public IEnumerable<CompilationFailure> CompilationFailures { get; }
 
         /// <summary>
         /// Gets the <see cref="CompilationResult"/>.
         /// </summary>
         /// <returns>The current <see cref="CompilationResult"/> instance.</returns>
         /// <exception cref="CompilationFailedException">Thrown if compilation failed.</exception>
-        public CompilationResult EnsureSuccessful()
+        public void EnsureSuccessful()
         {
             if (CompilationFailures != null)
             {
                 throw new CompilationFailedException(CompilationFailures);
             }
-
-            return this;
-        }
-
-        /// <summary>
-        /// Creates a <see cref="CompilationResult"/> for a failed compilation.
-        /// </summary>
-        /// <param name="compilationFailures"><see cref="CompilationFailure"/>s produced from parsing or
-        /// compiling the Razor file.</param>
-        /// <returns>A <see cref="CompilationResult"/> instance for a failed compilation.</returns>
-        public static CompilationResult Failed(IEnumerable<CompilationFailure> compilationFailures)
-        {
-            if (compilationFailures == null)
-            {
-                throw new ArgumentNullException(nameof(compilationFailures));
-            }
-
-            return new CompilationResult
-            {
-                CompilationFailures = compilationFailures
-            };
-        }
-
-        /// <summary>
-        /// Creates a <see cref="CompilationResult"/> for a successful compilation.
-        /// </summary>
-        /// <param name="type">The compiled type.</param>
-        /// <returns>A <see cref="CompilationResult"/> instance for a successful compilation.</returns>
-        public static CompilationResult Successful(Type type)
-        {
-            if (type == null)
-            {
-                throw new ArgumentNullException(nameof(type));
-            }
-
-            return new CompilationResult
-            {
-                CompiledType = type
-            };
         }
     }
 }

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

@@ -55,10 +55,9 @@ public CompilerCache(
                 throw new ArgumentNullException(nameof(precompiledViews));
             }
 
-            var expirationTokens = new IChangeToken[0];
             foreach (var item in precompiledViews)
             {
-                var cacheEntry = new CompilerCacheResult(CompilationResult.Successful(item.Value), expirationTokens);
+                var cacheEntry = new CompilerCacheResult(new CompilationResult(item.Value));
                 _cache.Set(GetNormalizedPath(item.Key), cacheEntry);
             }
         }
@@ -100,34 +99,26 @@ private CompilerCacheResult CreateCacheEntry(
             var fileInfo = _fileProvider.GetFileInfo(normalizedPath);
             MemoryCacheEntryOptions cacheEntryOptions;
             CompilerCacheResult cacheResult;
-            CompilerCacheResult cacheResultToCache;
             if (!fileInfo.Exists)
             {
                 var expirationToken = _fileProvider.Watch(normalizedPath);
                 cacheResult = new CompilerCacheResult(new[] { expirationToken });
-                cacheResultToCache = cacheResult;
 
                 cacheEntryOptions = new MemoryCacheEntryOptions();
                 cacheEntryOptions.AddExpirationToken(expirationToken);
             }
             else
             {
                 var relativeFileInfo = new RelativeFileInfo(fileInfo, normalizedPath);
-                var compilationResult = compile(relativeFileInfo).EnsureSuccessful();
+                var compilationResult = compile(relativeFileInfo);
+                compilationResult.EnsureSuccessful();
                 cacheEntryOptions = GetMemoryCacheEntryOptions(normalizedPath);
-
-                // By default the CompilationResult returned by IRoslynCompiler is an instance of
-                // UncachedCompilationResult. This type has the generated code as a string property and do not want
-                // to cache it. We'll instead cache the unwrapped result.
-                cacheResultToCache = new CompilerCacheResult(
-                    CompilationResult.Successful(compilationResult.CompiledType),
-                    cacheEntryOptions.ExpirationTokens);
                 cacheResult = new CompilerCacheResult(
                     compilationResult,
                     cacheEntryOptions.ExpirationTokens);
             }
 
-            _cache.Set(normalizedPath, cacheResultToCache, cacheEntryOptions);
+            _cache.Set(normalizedPath, cacheResult, cacheEntryOptions);
             return cacheResult;
         }
 

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

@@ -10,48 +10,69 @@ namespace Microsoft.AspNet.Mvc.Razor.Compilation
     /// <summary>
     /// Result of <see cref="ICompilerCache"/>.
     /// </summary>
-    public class CompilerCacheResult
+    public struct CompilerCacheResult
     {
         /// <summary>
         /// Initializes a new instance of <see cref="CompilerCacheResult"/> with the specified
-        /// <see cref="CompilationResult"/>.
+        /// <see cref="Compilation.CompilationResult"/>.
         /// </summary>
-        /// <param name="compilationResult">The <see cref="Compilation.CompilationResult"/> </param>
-        public CompilerCacheResult(CompilationResult compilationResult, IList<IChangeToken> expirationTokens)
+        /// <param name="compilationResult">The <see cref="Compilation.CompilationResult"/>.</param>
+        public CompilerCacheResult(CompilationResult compilationResult)
+            : this(compilationResult, new IChangeToken[0])
         {
-            if (compilationResult == null)
-            {
-                throw new ArgumentNullException(nameof(compilationResult));
-            }
+        }
 
+        /// <summary>
+        /// Initializes a new instance of <see cref="CompilerCacheResult"/> with the specified
+        /// <see cref="Compilation.CompilationResult"/>.
+        /// </summary>
+        /// <param name="compilationResult">The <see cref="Compilation.CompilationResult"/>.</param>
+        /// <param name="expirationTokens">One or more <see cref="IChangeToken"/> instances that indicate when
+        /// this result has expired.
+        public CompilerCacheResult(CompilationResult compilationResult, IList<IChangeToken> expirationTokens)
+        {
             if (expirationTokens == null)
             {
                 throw new ArgumentNullException(nameof(expirationTokens));
             }
 
             CompilationResult = compilationResult;
+            Success = true;
             ExpirationTokens = expirationTokens;
         }
 
+        /// <summary>
+        /// Initializes a new instance of <see cref="CompilerCacheResult"/> for a file that could not be
+        /// found in the file system.
+        /// </summary>
+        /// <param name="expirationTokens">One or more <see cref="IChangeToken"/> instances that indicate when
+        /// this result has expired.</param>
         public CompilerCacheResult(IList<IChangeToken> expirationTokens)
         {
             if (expirationTokens == null)
             {
                 throw new ArgumentNullException(nameof(expirationTokens));
             }
 
-            CompilationResult = null;
+            CompilationResult = default(CompilationResult);
+            Success = false;
             ExpirationTokens = expirationTokens;
         }
 
         /// <summary>
         /// The <see cref="Compilation.CompilationResult"/>.
         /// </summary>
-        /// <remarks>This property is null when file lookup failed.</remarks>
+        /// <remarks>This property is not available when <see cref="Success"/> is <c>false</c>.</remarks>
         public CompilationResult CompilationResult { get; }
 
+        /// <summary>
+        /// <see cref="IChangeToken"/> instances that indicate when this result has expired.
+        /// </summary>
         public IList<IChangeToken> ExpirationTokens { get; }
 
-        public bool IsFoundResult => CompilationResult != null;
+        /// <summary>
+        /// Gets a value that determines if the view was successfully found and compiled.
+        /// </summary>
+        public bool Success { get; }
     }
 }

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

@@ -100,7 +100,7 @@ internal CompilationResult GetCompilationFailedResult(RelativeFileInfo file, IEn
                 failures.Add(compilationFailure);
             }
 
-            return CompilationResult.Failed(failures);
+            return new CompilationResult(failures);
         }
 
         private DiagnosticMessage CreateDiagnosticMessage(RazorError error, string filePath)

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

@@ -149,7 +149,7 @@ public CompilationResult Compile(RelativeFileInfo fileInfo, string compilationCo
                     var type = assembly.GetExportedTypes()
                                        .First(t => t.Name.StartsWith(_classPrefix, StringComparison.Ordinal));
 
-                    return UncachedCompilationResult.Successful(type, compilationContent);
+                    return new CompilationResult(type);
                 }
             }
         }
@@ -214,7 +214,7 @@ internal CompilationResult GetCompilationFailedResult(
                 failures.Add(compilationFailure);
             }
 
-            return CompilationResult.Failed(failures);
+            return new CompilationResult(failures);
         }
 
         private static string GetFilePath(string relativePath, Diagnostic diagnostic)

src/Microsoft.AspNet.Mvc.Razor/DefaultRazorPageFactory.cs → src/Microsoft.AspNet.Mvc.Razor/DefaultRazorPageFactoryProvider.cs

@@ -7,10 +7,10 @@
 namespace Microsoft.AspNet.Mvc.Razor
 {
     /// <summary>
-    /// Represents a <see cref="IRazorPageFactory"/> that creates <see cref="RazorPage"/> instances
+    /// Represents a <see cref="IRazorPageFactoryProvider"/> that creates <see cref="RazorPage"/> instances
     /// from razor files in the file system.
     /// </summary>
-    public class DefaultRazorPageFactory : IRazorPageFactory
+    public class DefaultRazorPageFactoryProvider : IRazorPageFactoryProvider
     {
         /// <remarks>
         /// This delegate holds on to an instance of <see cref="IRazorCompilationService"/>.
@@ -24,7 +24,7 @@ public class DefaultRazorPageFactory : IRazorPageFactory
         /// </summary>
         /// <param name="razorCompilationService">The <see cref="IRazorCompilationService"/>.</param>
         /// <param name="compilerCacheProvider">The <see cref="ICompilerCacheProvider"/>.</param>
-        public DefaultRazorPageFactory(
+        public DefaultRazorPageFactoryProvider(
             IRazorCompilationService razorCompilationService,
             ICompilerCacheProvider compilerCacheProvider)
         {
@@ -59,19 +59,32 @@ public RazorPageFactoryResult CreateFactory(string relativePath)
                 relativePath = relativePath.Substring(1);
             }
             var result = CompilerCache.GetOrAdd(relativePath, _compileDelegate);
-            if (result.IsFoundResult)
+            if (result.Success)
             {
-                return new RazorPageFactoryResult(() =>
-                {
-                    var page = (IRazorPage)Activator.CreateInstance(result.CompilationResult.CompiledType);
-                    page.Path = relativePath;
-                    return page;
-                }, result.ExpirationTokens);
+                var pageFactory = GetPageFactory(result.CompilationResult.CompiledType, relativePath);
+                return new RazorPageFactoryResult(pageFactory, result.ExpirationTokens);
             }
             else
             {
                 return new RazorPageFactoryResult(result.ExpirationTokens);
             }
         }
+
+        /// <summary>
+        /// Creates a factory for <see cref="IRazorPage"/>.
+        /// </summary>
+        /// <param name="compiledType">The <see cref="Type"/> to produce an instance of <see cref="IRazorPage"/>
+        /// from.</param>
+        /// <param name="relativePath">The application relative path of the page.</param>
+        /// <returns>A factory for <paramref name="compiledType"/>.</returns>
+        protected virtual Func<IRazorPage> GetPageFactory(Type compiledType, string relativePath)
+        {
+            return () =>
+            {
+                var page = (IRazorPage)Activator.CreateInstance(compiledType);
+                page.Path = relativePath;
+                return page;
+            };
+        }
     }
 }

src/Microsoft.AspNet.Mvc.Razor/DependencyInjection/MvcRazorMvcCoreBuilderExtensions.cs

@@ -145,8 +145,7 @@ internal static void AddRazorViewEngineServices(IServiceCollection services)
 
             // In the default scenario the following services are singleton by virtue of being initialized as part of
             // creating the singleton RazorViewEngine instance.
-            services.TryAddTransient<IRazorViewFactory, RazorViewFactory>();
-            services.TryAddTransient<IRazorPageFactory, DefaultRazorPageFactory>();
+            services.TryAddTransient<IRazorPageFactoryProvider, DefaultRazorPageFactoryProvider>();
             services.TryAddTransient<IRazorCompilationService, RazorCompilationService>();
             services.TryAddTransient<IMvcRazorHost, MvcRazorHost>();