From 74da350086cb148adbee81e501139ce45a6e74bc Mon Sep 17 00:00:00 2001 From: Pranav K Date: Tue, 28 Oct 2014 15:34:09 -0700 Subject: [PATCH] Remove enableInstrumentation from CompilerCache and IMvcRazorHost --- .../IMvcRazorHost.cs | 5 -- .../MvcRazorHost.cs | 2 + .../Compilation/CompilerCache.cs | 19 +++----- .../Compilation/CompilerCacheEntry.cs | 12 +---- .../Compilation/ICompilerCache.cs | 2 - .../Razor/IRazorCompilationService.cs | 3 +- .../Razor/PreCompileViews/RazorPreCompiler.cs | 1 - .../Razor/RazorCompilationService.cs | 4 +- .../VirtualPathRazorPageFactory.cs | 8 +--- .../MvcRazorHostTest.cs | 23 ++++++++-- .../TestFiles/Output/Runtime/Basic.cs | 8 ++++ .../Runtime/ModelExpressionTagHelper.cs | 2 + .../Compilation/CompilerCacheTest.cs | 46 ++----------------- .../RazorCompilationServiceTest.cs | 35 +------------- 14 files changed, 47 insertions(+), 123 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Razor.Host/IMvcRazorHost.cs b/src/Microsoft.AspNet.Mvc.Razor.Host/IMvcRazorHost.cs index 79a0b2d700..74b11d11ff 100644 --- a/src/Microsoft.AspNet.Mvc.Razor.Host/IMvcRazorHost.cs +++ b/src/Microsoft.AspNet.Mvc.Razor.Host/IMvcRazorHost.cs @@ -11,11 +11,6 @@ namespace Microsoft.AspNet.Mvc.Razor /// public interface IMvcRazorHost { - /// - /// Flag that indicates if page execution instrumentation code should be injected into the output. - /// - bool EnableInstrumentation { get; set; } - /// /// Parses and generates the contents of a Razor file represented by . /// diff --git a/src/Microsoft.AspNet.Mvc.Razor.Host/MvcRazorHost.cs b/src/Microsoft.AspNet.Mvc.Razor.Host/MvcRazorHost.cs index d4112923fa..c0063996fc 100644 --- a/src/Microsoft.AspNet.Mvc.Razor.Host/MvcRazorHost.cs +++ b/src/Microsoft.AspNet.Mvc.Razor.Host/MvcRazorHost.cs @@ -62,6 +62,8 @@ namespace Microsoft.AspNet.Mvc.Razor TagHelperDescriptorResolver = new TagHelperDescriptorResolver(); DefaultBaseClass = BaseType + '<' + DefaultModel + '>'; DefaultNamespace = "Asp"; + // Enable instrumentation by default to allow precompiled views to work with BrowserLink. + EnableInstrumentation = true; GeneratedClassContext = new GeneratedClassContext( executeMethodName: "ExecuteAsync", writeMethodName: "Write", diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs index 4618542916..1445ae10b5 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCache.cs @@ -74,24 +74,19 @@ namespace Microsoft.AspNet.Mvc.Razor /// public CompilationResult GetOrAdd([NotNull] RelativeFileInfo fileInfo, - bool enableInstrumentation, [NotNull] Func compile) { - CompilerCacheEntry cacheEntry; if (!_cache.TryGetValue(NormalizePath(fileInfo.RelativePath), out cacheEntry)) { - return OnCacheMiss(fileInfo, enableInstrumentation, compile); + return OnCacheMiss(fileInfo, compile); } else { - if ((cacheEntry.Length != fileInfo.FileInfo.Length) || - (enableInstrumentation && !cacheEntry.IsInstrumented)) + if (cacheEntry.Length != fileInfo.FileInfo.Length) { - // Recompile if - // (a) If the file lengths differ - // (b) If the compiled type is not instrumented but we require it to be instrumented. - return OnCacheMiss(fileInfo, enableInstrumentation, compile); + // Recompile if the file lengths differ + return OnCacheMiss(fileInfo, compile); } if (cacheEntry.LastModified == fileInfo.FileInfo.LastModified) @@ -108,22 +103,20 @@ namespace Microsoft.AspNet.Mvc.Razor { // Cache hit, but we need to update the entry return OnCacheMiss(fileInfo, - enableInstrumentation, () => CompilationResult.Successful(cacheEntry.CompiledType)); } // it's not a match, recompile - return OnCacheMiss(fileInfo, enableInstrumentation, compile); + return OnCacheMiss(fileInfo, compile); } } private CompilationResult OnCacheMiss(RelativeFileInfo file, - bool isInstrumented, Func compile) { var result = compile(); - var cacheEntry = new CompilerCacheEntry(file, result.CompiledType, isInstrumented); + var cacheEntry = new CompilerCacheEntry(file, result.CompiledType); _cache[NormalizePath(file.RelativePath)] = cacheEntry; return result; diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCacheEntry.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCacheEntry.cs index 90e2881182..4025a5c6bc 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCacheEntry.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/CompilerCacheEntry.cs @@ -22,8 +22,6 @@ namespace Microsoft.AspNet.Mvc.Razor Length = info.Length; LastModified = info.LastModified; Hash = info.Hash; - // Precompiled views are always instrumented. - IsInstrumented = true; } /// @@ -31,15 +29,12 @@ namespace Microsoft.AspNet.Mvc.Razor /// /// Metadata about the file that was compiled. /// The compiled . - /// Flag that indicates that the file was generated with instrumentation - /// enabled. - public CompilerCacheEntry([NotNull] RelativeFileInfo info, [NotNull] Type compiledType, bool isInstrumented) + public CompilerCacheEntry([NotNull] RelativeFileInfo info, [NotNull] Type compiledType) { CompiledType = compiledType; RelativePath = info.RelativePath; Length = info.FileInfo.Length; LastModified = info.FileInfo.LastModified; - IsInstrumented = isInstrumented; } /// @@ -71,10 +66,5 @@ namespace Microsoft.AspNet.Mvc.Razor /// Gets a flag that indicates if the file is precompiled. /// public bool IsPreCompiled { get { return Hash != null; } } - - /// - /// Gets a flag that indiciates if the page execution in is instrumeted. - /// - public bool IsInstrumented { get; private set; } } } diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs index 3d06c9ab89..474dddb4fd 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/ICompilerCache.cs @@ -15,11 +15,9 @@ namespace Microsoft.AspNet.Mvc.Razor /// 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); } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Razor/Razor/IRazorCompilationService.cs b/src/Microsoft.AspNet.Mvc.Razor/Razor/IRazorCompilationService.cs index a56e9eb146..2ebc4e4d90 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Razor/IRazorCompilationService.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Razor/IRazorCompilationService.cs @@ -13,10 +13,9 @@ 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. /// - CompilationResult Compile(RelativeFileInfo fileInfo, bool isInstrumented); + CompilationResult Compile(RelativeFileInfo fileInfo); } } diff --git a/src/Microsoft.AspNet.Mvc.Razor/Razor/PreCompileViews/RazorPreCompiler.cs b/src/Microsoft.AspNet.Mvc.Razor/Razor/PreCompileViews/RazorPreCompiler.cs index 2c72199bc4..4d793366ca 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Razor/PreCompileViews/RazorPreCompiler.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Razor/PreCompileViews/RazorPreCompiler.cs @@ -32,7 +32,6 @@ namespace Microsoft.AspNet.Mvc.Razor { _serviceProvider = designTimeServiceProvider; _host = host; - _host.EnableInstrumentation = true; var appEnv = _serviceProvider.GetRequiredService(); _fileSystem = optionsAccessor.Options.FileSystem; diff --git a/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs b/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs index baa5c820e1..51a317b0c5 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Razor/RazorCompilationService.cs @@ -22,10 +22,8 @@ namespace Microsoft.AspNet.Mvc.Razor } /// - public CompilationResult Compile([NotNull] RelativeFileInfo file, bool isInstrumented) + public CompilationResult Compile([NotNull] RelativeFileInfo file) { - _razorHost.EnableInstrumentation = isInstrumented; - GeneratorResults results; using (var inputStream = file.FileInfo.CreateReadStream()) { diff --git a/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs b/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs index a407538ac7..95af0230de 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/VirtualPathRazorPageFactory.cs @@ -18,20 +18,17 @@ namespace Microsoft.AspNet.Mvc.Razor private readonly IServiceProvider _serviceProvider; private readonly IFileInfoCache _fileInfoCache; private readonly ICompilerCache _compilerCache; - private readonly bool _isInstrumentationEnabled; private IRazorCompilationService _razorcompilationService; public VirtualPathRazorPageFactory(ITypeActivator typeActivator, IServiceProvider serviceProvider, ICompilerCache compilerCache, - IFileInfoCache fileInfoCache, - IContextAccessor contextAccessor) + IFileInfoCache fileInfoCache) { _activator = typeActivator; _serviceProvider = serviceProvider; _compilerCache = compilerCache; _fileInfoCache = fileInfoCache; - _isInstrumentationEnabled = IsInstrumentationEnabled(contextAccessor.Value); } private IRazorCompilationService RazorCompilationService @@ -71,8 +68,7 @@ namespace Microsoft.AspNet.Mvc.Razor var result = _compilerCache.GetOrAdd( relativeFileInfo, - _isInstrumentationEnabled, - () => RazorCompilationService.Compile(relativeFileInfo, _isInstrumentationEnabled)); + () => RazorCompilationService.Compile(relativeFileInfo)); var page = (IRazorPage)_activator.CreateInstance(_serviceProvider, result.CompiledType); page.Path = relativePath; diff --git a/test/Microsoft.AspNet.Mvc.Razor.Host.Test/MvcRazorHostTest.cs b/test/Microsoft.AspNet.Mvc.Razor.Host.Test/MvcRazorHostTest.cs index a51630bb22..cac8962cb9 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Host.Test/MvcRazorHostTest.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Host.Test/MvcRazorHostTest.cs @@ -12,6 +12,19 @@ namespace Microsoft.AspNet.Mvc.Razor { public class MvcRazorHostTest { + [Fact] + public void MvcRazorHost_EnablesInstrumentationByDefault() + { + // Arrange + var host = new MvcRazorHost(new TestFileSystem()); + + // Act + var instrumented = host.EnableInstrumentation; + + // Assert + Assert.True(instrumented); + } + [Fact] public void MvcRazorHost_GeneratesTagHelperModelExpressionCode_DesignTime() { @@ -22,12 +35,12 @@ namespace Microsoft.AspNet.Mvc.Razor }; var expectedLineMappings = new List { - BuildLineMapping(documentAbsoluteIndex: 7, + BuildLineMapping(documentAbsoluteIndex: 7, documentLineIndex: 0, - documentCharacterIndex: 7, + documentCharacterIndex: 7, generatedAbsoluteIndex: 444, generatedLineIndex: 12, - generatedCharacterIndex: 7, + generatedCharacterIndex: 7, contentLength: 8), BuildLineMapping(documentAbsoluteIndex: 33, documentLineIndex: 2, @@ -39,8 +52,8 @@ namespace Microsoft.AspNet.Mvc.Razor }; // Act and Assert - RunDesignTimeTest(host, - testName: "ModelExpressionTagHelper", + RunDesignTimeTest(host, + testName: "ModelExpressionTagHelper", expectedLineMappings: expectedLineMappings); } diff --git a/test/Microsoft.AspNet.Mvc.Razor.Host.Test/TestFiles/Output/Runtime/Basic.cs b/test/Microsoft.AspNet.Mvc.Razor.Host.Test/TestFiles/Output/Runtime/Basic.cs index f32f058b6f..99120136ef 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Host.Test/TestFiles/Output/Runtime/Basic.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Host.Test/TestFiles/Output/Runtime/Basic.cs @@ -27,16 +27,24 @@ namespace Asp #pragma warning disable 1998 public override async Task ExecuteAsync() { + BeginContext(0, 4, true); WriteLiteral("(logo, 12), false)); + BeginContext(18, 24, true); WriteLiteral(">\r\n Hello world\r\n "); + EndContext(); + BeginContext(43, 21, false); #line 3 "TestFiles/Input/Basic.cshtml" Write(Html.Input("SomeKey")); #line default #line hidden + EndContext(); + BeginContext(64, 8, true); WriteLiteral("\r\n"); + EndContext(); } #pragma warning restore 1998 } diff --git a/test/Microsoft.AspNet.Mvc.Razor.Host.Test/TestFiles/Output/Runtime/ModelExpressionTagHelper.cs b/test/Microsoft.AspNet.Mvc.Razor.Host.Test/TestFiles/Output/Runtime/ModelExpressionTagHelper.cs index 2687d004cd..581614783e 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Host.Test/TestFiles/Output/Runtime/ModelExpressionTagHelper.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Host.Test/TestFiles/Output/Runtime/ModelExpressionTagHelper.cs @@ -40,7 +40,9 @@ namespace Asp #pragma warning disable 1998 public override async Task ExecuteAsync() { + BeginContext(120, 2, true); WriteLiteral("\r\n"); + EndContext(); __tagHelperExecutionContext = __tagHelperScopeManager.Begin("inputTest"); __Microsoft_AspNet_Mvc_Razor_InputTestTagHelper = CreateTagHelper(); __tagHelperExecutionContext.Add(__Microsoft_AspNet_Mvc_Razor_InputTestTagHelper); diff --git a/test/Microsoft.AspNet.Mvc.Razor.Test/Compilation/CompilerCacheTest.cs b/test/Microsoft.AspNet.Mvc.Razor.Test/Compilation/CompilerCacheTest.cs index d11654d5df..9dad480b21 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Test/Compilation/CompilerCacheTest.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Test/Compilation/CompilerCacheTest.cs @@ -34,7 +34,7 @@ namespace Microsoft.AspNet.Mvc.Razor.Test }; // Act - var actual = cache.GetOrAdd(runtimeFileInfo, false, () => expected); + var actual = cache.GetOrAdd(runtimeFileInfo, () => expected); // Assert Assert.Same(expected, actual); @@ -140,8 +140,7 @@ namespace Microsoft.AspNet.Mvc.Razor.Test // Act var actual = cache.GetOrAdd(runtimeFileInfo, - enableInstrumentation: false, - compile: () => CompilationResult.Successful(resultViewType)); + compile: () => CompilationResult.Successful(resultViewType)); // Assert if (swapsPreCompile) @@ -175,9 +174,9 @@ namespace Microsoft.AspNet.Mvc.Razor.Test }; // Act - cache.GetOrAdd(runtimeFileInfo, false, () => uncachedResult); - var actual1 = cache.GetOrAdd(runtimeFileInfo, false, () => uncachedResult); - var actual2 = cache.GetOrAdd(runtimeFileInfo, false, () => uncachedResult); + cache.GetOrAdd(runtimeFileInfo, () => uncachedResult); + var actual1 = cache.GetOrAdd(runtimeFileInfo, () => uncachedResult); + var actual2 = cache.GetOrAdd(runtimeFileInfo, () => uncachedResult); // Assert Assert.NotSame(uncachedResult, actual1); @@ -190,40 +189,5 @@ namespace Microsoft.AspNet.Mvc.Razor.Test Assert.Null(actual2.CompiledContent); Assert.Same(type, actual2.CompiledType); } - - [Fact] - public void GetOrAdd_IgnoresCache_IfCachedItemIsNotInstrumentedAndEnableInstrumentationIsTrue() - { - // Arrange - var lastModified = DateTime.UtcNow; - var cache = new CompilerCache(); - var fileInfo = new Mock(); - fileInfo.SetupGet(f => f.PhysicalPath) - .Returns("test"); - fileInfo.SetupGet(f => f.LastModified) - .Returns(lastModified); - var type = GetType(); - var uncachedResult1 = UncachedCompilationResult.Successful(type, "hello world"); - var uncachedResult2 = UncachedCompilationResult.Successful(typeof(object), "hello world"); - var uncachedResult3 = UncachedCompilationResult.Successful(typeof(Guid), "hello world"); - - var runtimeFileInfo = new RelativeFileInfo() - { - FileInfo = fileInfo.Object, - RelativePath = "test", - }; - - // Act - cache.GetOrAdd(runtimeFileInfo, false, () => uncachedResult1); - var actual1 = cache.GetOrAdd(runtimeFileInfo, true, () => uncachedResult2); - var actual2 = cache.GetOrAdd(runtimeFileInfo, false, () => uncachedResult3); - - // Assert - Assert.Same(uncachedResult2, actual1); - Assert.Same(typeof(object), actual1.CompiledType); - - Assert.NotSame(actual2, uncachedResult3); - Assert.Same(typeof(object), actual2.CompiledType); - } } } \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.Razor.Test/RazorCompilationServiceTest.cs b/test/Microsoft.AspNet.Mvc.Razor.Test/RazorCompilationServiceTest.cs index 87086c76bd..8338be5dd8 100644 --- a/test/Microsoft.AspNet.Mvc.Razor.Test/RazorCompilationServiceTest.cs +++ b/test/Microsoft.AspNet.Mvc.Razor.Test/RazorCompilationServiceTest.cs @@ -1,10 +1,7 @@ // 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.IO; -using System.Linq; -using System.Reflection; using Microsoft.AspNet.FileSystems; using Microsoft.AspNet.Razor; using Microsoft.AspNet.Razor.Generator.Compiler; @@ -44,42 +41,12 @@ namespace Microsoft.AspNet.Mvc.Razor.Test }; // Act - razorService.Compile(relativeFileInfo, isInstrumented: false); + razorService.Compile(relativeFileInfo); // Assert host.Verify(); } - [Theory] - [InlineData(false)] - [InlineData(true)] - public void CompileSetsEnableInstrumentationOnHost(bool enableInstrumentation) - { - // Arrange - var host = new Mock(); - host.SetupAllProperties(); - host.Setup(h => h.GenerateCode(It.IsAny(), It.IsAny())) - .Returns(GetGeneratorResult()); - - var compiler = new Mock(); - compiler.Setup(c => c.Compile(It.IsAny(), It.IsAny())) - .Returns(CompilationResult.Successful(GetType())); - - var razorService = new RazorCompilationService(compiler.Object, host.Object); - - var relativeFileInfo = new RelativeFileInfo() - { - FileInfo = Mock.Of(), - RelativePath = @"views\index\home.cshtml", - }; - - // Act - razorService.Compile(relativeFileInfo, isInstrumented: enableInstrumentation); - - // Assert - Assert.Equal(enableInstrumentation, host.Object.EnableInstrumentation); - } - private static GeneratorResults GetGeneratorResult() { return new GeneratorResults(