From 304000095a7996f37f0185af16eb7a671280a8ab Mon Sep 17 00:00:00 2001 From: Jass Bagga Date: Mon, 7 Nov 2016 10:46:54 -0800 Subject: [PATCH] Added logging for precompiled views in RazorViewEngine Addresses #5462 --- .../Internal/CompilerCache.cs | 2 +- .../Internal/CompilerCacheResult.cs | 22 +++++++++++++- .../DefaultRazorPageFactoryProvider.cs | 2 +- .../Internal/MvcRazorLoggerExtensions.cs | 11 +++++++ .../RazorPageFactoryResult.cs | 22 ++++++++++++++ .../RazorViewEngine.cs | 7 ++++- .../Internal/CompilerCacheTest.cs | 1 + .../RazorViewEngineTest.cs | 29 +++++++++++++++++++ 8 files changed, 92 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/Internal/CompilerCache.cs b/src/Microsoft.AspNetCore.Mvc.Razor/Internal/CompilerCache.cs index 0a08695649..5a91909aae 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/Internal/CompilerCache.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/Internal/CompilerCache.cs @@ -59,7 +59,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal foreach (var item in views) { - var cacheEntry = new CompilerCacheResult(item.Key, new CompilationResult(item.Value)); + var cacheEntry = new CompilerCacheResult(item.Key, new CompilationResult(item.Value), isPrecompiled: true); _cache.Set(GetNormalizedPath(item.Key), Task.FromResult(cacheEntry)); } } diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/Internal/CompilerCacheResult.cs b/src/Microsoft.AspNetCore.Mvc.Razor/Internal/CompilerCacheResult.cs index c889b23406..df8a222434 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/Internal/CompilerCacheResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/Internal/CompilerCacheResult.cs @@ -27,6 +27,19 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal { } + /// + /// Initializes a new instance of with the specified + /// . + /// + /// Path of the view file relative to the application base. + /// The . + /// true if the view is precompiled, false otherwise. + public CompilerCacheResult(string relativePath, CompilationResult compilationResult, bool isPrecompiled) + : this(relativePath, compilationResult, EmptyArray.Instance) + { + IsPrecompiled = isPrecompiled; + } + /// /// Initializes a new instance of with the specified /// . @@ -51,9 +64,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal var propertyBindExpression = Expression.Bind(pathProperty, Expression.Constant(relativePath)); var objectInitializeExpression = Expression.MemberInit(newExpression, propertyBindExpression); - PageFactory = Expression + PageFactory = Expression .Lambda>(objectInitializeExpression) .Compile(); + IsPrecompiled = false; } /// @@ -71,6 +85,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal ExpirationTokens = expirationTokens; PageFactory = null; + IsPrecompiled = false; } /// @@ -87,5 +102,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal /// Gets a delegate that creates an instance of the . /// public Func PageFactory { get; } + + /// + /// Gets a value that determines if the view is precompiled. + /// + public bool IsPrecompiled { get; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/Internal/DefaultRazorPageFactoryProvider.cs b/src/Microsoft.AspNetCore.Mvc.Razor/Internal/DefaultRazorPageFactoryProvider.cs index dc371c835a..4b26111089 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/Internal/DefaultRazorPageFactoryProvider.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/Internal/DefaultRazorPageFactoryProvider.cs @@ -62,7 +62,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal var result = CompilerCache.GetOrAdd(relativePath, _compileDelegate); if (result.Success) { - return new RazorPageFactoryResult(result.PageFactory, result.ExpirationTokens); + return new RazorPageFactoryResult(result.PageFactory, result.ExpirationTokens, result.IsPrecompiled); } else { diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/Internal/MvcRazorLoggerExtensions.cs b/src/Microsoft.AspNetCore.Mvc.Razor/Internal/MvcRazorLoggerExtensions.cs index cf6fff2865..2342938e6c 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/Internal/MvcRazorLoggerExtensions.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/Internal/MvcRazorLoggerExtensions.cs @@ -19,6 +19,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal private static readonly Action _viewLookupCacheMiss; private static readonly Action _viewLookupCacheHit; + private static readonly Action _precompiledViewFound; static MvcRazorLoggerExtensions() { @@ -42,6 +43,11 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal 2, "View lookup cache hit for view '{ViewName}' in controller '{ControllerName}'."); + _precompiledViewFound = LoggerMessage.Define( + LogLevel.Debug, + 3, + "Using precompiled view for '{RelativePath}'."); + _generatedCodeToAssemblyCompilationStart = LoggerMessage.Define( LogLevel.Debug, 1, @@ -79,6 +85,11 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal _viewLookupCacheHit(logger, viewName, controllerName, null); } + public static void PrecompiledViewFound(this ILogger logger, string relativePath) + { + _precompiledViewFound(logger, relativePath, null); + } + public static void GeneratedCodeToAssemblyCompilationStart(this ILogger logger, string filePath) { _generatedCodeToAssemblyCompilationStart(logger, filePath, null); diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/RazorPageFactoryResult.cs b/src/Microsoft.AspNetCore.Mvc.Razor/RazorPageFactoryResult.cs index 3e7ab71f53..1d430ca4da 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/RazorPageFactoryResult.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/RazorPageFactoryResult.cs @@ -26,6 +26,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor ExpirationTokens = expirationTokens; RazorPageFactory = null; + IsPrecompiled = false; } /// @@ -37,6 +38,21 @@ namespace Microsoft.AspNetCore.Mvc.Razor public RazorPageFactoryResult( Func razorPageFactory, IList expirationTokens) + : this(razorPageFactory, expirationTokens, isPrecompiled: false) + { + } + + /// + /// Initializes a new instance of with the + /// specified factory. + /// + /// The factory. + /// One or more instances. + /// true if the view is precompiled, false otherwise. + public RazorPageFactoryResult( + Func razorPageFactory, + IList expirationTokens, + bool isPrecompiled) { if (razorPageFactory == null) { @@ -50,6 +66,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor RazorPageFactory = razorPageFactory; ExpirationTokens = expirationTokens; + IsPrecompiled = isPrecompiled; } /// @@ -68,5 +85,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor /// Gets a value that determines if the page was successfully located. /// public bool Success => RazorPageFactory != null; + + /// + /// Gets a value that determines if the view is precompiled. + /// + public bool IsPrecompiled { get; } } } diff --git a/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs b/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs index b3dfab2373..8eb2df6a16 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor/RazorViewEngine.cs @@ -395,7 +395,8 @@ namespace Microsoft.AspNetCore.Mvc.Razor return ViewLookupCache.Set(cacheKey, cacheResult, cacheEntryOptions); } - private ViewLocationCacheResult CreateCacheResult( + // Internal for unit testing + internal ViewLocationCacheResult CreateCacheResult( HashSet expirationTokens, string relativePath, bool isMainPage) @@ -415,6 +416,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor var viewStartPages = isMainPage ? GetViewStartPages(relativePath, expirationTokens) : EmptyArray.Instance; + if (factoryResult.IsPrecompiled) + { + _logger.PrecompiledViewFound(relativePath); + } return new ViewLocationCacheResult( new ViewLocationCacheItem(factoryResult.RazorPageFactory, relativePath), diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Test/Internal/CompilerCacheTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Test/Internal/CompilerCacheTest.cs index cf525a6443..5e5e4885b1 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Test/Internal/CompilerCacheTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Test/Internal/CompilerCacheTest.cs @@ -241,6 +241,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Internal // Assert Assert.True(result.Success); + Assert.True(result.IsPrecompiled); Assert.IsType(result.PageFactory()); } diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs index 5407c4dcf4..5b8869680b 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Test/RazorViewEngineTest.cs @@ -1283,6 +1283,35 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Test Assert.Equal(expected, result.SearchedLocations); } + [Fact] + public void CreateCacheResult_LogsPrecompiledViewFound() + { + // Arrange + var sink = new TestSink(); + var loggerFactory = new TestLoggerFactory(sink, enabled: true); + + var relativePath = "/Views/Foo/details.cshtml"; + var pageFactory = new Mock(); + pageFactory + .Setup(p => p.CreateFactory(relativePath)) + .Returns(new RazorPageFactoryResult(() => Mock.Of(), new IChangeToken[0], isPrecompiled: true)) + .Verifiable(); + + var viewEngine = new RazorViewEngine( + pageFactory.Object, + Mock.Of(), + new HtmlTestEncoder(), + GetOptionsAccessor(expanders: null), + loggerFactory); + + // Act + var result = viewEngine.CreateCacheResult(null, relativePath, false); + + // Assert + var logMessage = Assert.Single(sink.Writes); + Assert.Equal($"Using precompiled view for '{relativePath}'.", logMessage.State.ToString()); + } + [Theory] [InlineData("/Test-View.cshtml")] [InlineData("~/Test-View.CSHTML")]