From 3375fc8b998e5e40d5080abd6f3c1cedd18892c9 Mon Sep 17 00:00:00 2001 From: "N. Taylor Mullen" Date: Fri, 9 Feb 2018 16:39:35 -0800 Subject: [PATCH] Change RazorProjectEngine to operate on RazorProjectItems. - Removed the `Process(string)` overload to make it extra clear that you must operate on project items. This way we also don't need to worry about the various formats of paths that can flow through the system. - Updated tests to use the new project item format. - Did a few formatting fixes on unrealted files. #2049 --- .../DefaultMvcImportFeature.cs | 12 ++++---- .../DefaultMvcImportFeature.cs | 12 ++++---- .../DefaultRazorImportFeature.cs | 2 +- .../DefaultRazorProjectEngine.cs | 23 ++++----------- .../IRazorImportFeature.cs | 2 +- .../RazorProjectEngine.cs | 4 +-- .../RazorProjectEngineFeatureBase.cs | 2 +- .../VirtualRazorProjectFileSystem.cs | 9 +++--- .../DefaultMvcImportFeatureTest.cs | 13 ++++----- .../DefaultMvcImportFeatureTest.cs | 13 ++++----- ...efaultRazorProjectEngineIntegrationTest.cs | 10 +++---- .../DefaultRazorProjectEngineTest.cs | 29 ------------------- 12 files changed, 42 insertions(+), 89 deletions(-) delete mode 100644 test/Microsoft.AspNetCore.Razor.Language.Test/DefaultRazorProjectEngineTest.cs diff --git a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X/DefaultMvcImportFeature.cs b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X/DefaultMvcImportFeature.cs index d630a70c72..055f306eb5 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X/DefaultMvcImportFeature.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X/DefaultMvcImportFeature.cs @@ -14,18 +14,18 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X { private const string ImportsFileName = "_ViewImports.cshtml"; - public IReadOnlyList GetImports(string sourceFilePath) + public IReadOnlyList GetImports(RazorProjectItem projectItem) { - if (string.IsNullOrEmpty(sourceFilePath)) + if (projectItem == null) { - throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpy, nameof(sourceFilePath)); + throw new ArgumentNullException(nameof(projectItem)); } var imports = new List(); AddDefaultDirectivesImport(imports); // We add hierarchical imports second so any default directive imports can be overridden. - AddHierarchicalImports(sourceFilePath, imports); + AddHierarchicalImports(projectItem, imports); return imports; } @@ -58,10 +58,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X } // Internal for testing - internal void AddHierarchicalImports(string sourceFilePath, List imports) + internal void AddHierarchicalImports(RazorProjectItem projectItem, List imports) { // We want items in descending order. FindHierarchicalItems returns items in ascending order. - var importProjectItems = ProjectEngine.FileSystem.FindHierarchicalItems(sourceFilePath, ImportsFileName).Reverse(); + var importProjectItems = ProjectEngine.FileSystem.FindHierarchicalItems(projectItem.FilePath, ImportsFileName).Reverse(); foreach (var importProjectItem in importProjectItems) { RazorSourceDocument importSourceDocument; diff --git a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/DefaultMvcImportFeature.cs b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/DefaultMvcImportFeature.cs index 8cbf0f11a1..4852951e33 100644 --- a/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/DefaultMvcImportFeature.cs +++ b/src/Microsoft.AspNetCore.Mvc.Razor.Extensions/DefaultMvcImportFeature.cs @@ -14,18 +14,18 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions { private const string ImportsFileName = "_ViewImports.cshtml"; - public IReadOnlyList GetImports(string sourceFilePath) + public IReadOnlyList GetImports(RazorProjectItem projectItem) { - if (string.IsNullOrEmpty(sourceFilePath)) + if (projectItem == null) { - throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpy, nameof(sourceFilePath)); + throw new ArgumentNullException(nameof(projectItem)); } var imports = new List(); AddDefaultDirectivesImport(imports); // We add hierarchical imports second so any default directive imports can be overridden. - AddHierarchicalImports(sourceFilePath, imports); + AddHierarchicalImports(projectItem, imports); return imports; } @@ -60,10 +60,10 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions } // Internal for testing - internal void AddHierarchicalImports(string sourceFilePath, List imports) + internal void AddHierarchicalImports(RazorProjectItem projectItem, List imports) { // We want items in descending order. FindHierarchicalItems returns items in ascending order. - var importProjectItems = ProjectEngine.FileSystem.FindHierarchicalItems(sourceFilePath, ImportsFileName).Reverse(); + var importProjectItems = ProjectEngine.FileSystem.FindHierarchicalItems(projectItem.FilePath, ImportsFileName).Reverse(); foreach (var importProjectItem in importProjectItems) { RazorSourceDocument importSourceDocument; diff --git a/src/Microsoft.AspNetCore.Razor.Language/DefaultRazorImportFeature.cs b/src/Microsoft.AspNetCore.Razor.Language/DefaultRazorImportFeature.cs index 3d5c015a26..ea626e97e0 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/DefaultRazorImportFeature.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/DefaultRazorImportFeature.cs @@ -8,6 +8,6 @@ namespace Microsoft.AspNetCore.Razor.Language { internal class DefaultRazorImportFeature : RazorProjectEngineFeatureBase, IRazorImportFeature { - public IReadOnlyList GetImports(string sourceFilePath) => Array.Empty(); + public IReadOnlyList GetImports(RazorProjectItem projectItem) => Array.Empty(); } } diff --git a/src/Microsoft.AspNetCore.Razor.Language/DefaultRazorProjectEngine.cs b/src/Microsoft.AspNetCore.Razor.Language/DefaultRazorProjectEngine.cs index 8f3017cea1..6cac02dc25 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/DefaultRazorProjectEngine.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/DefaultRazorProjectEngine.cs @@ -45,29 +45,16 @@ namespace Microsoft.AspNetCore.Razor.Language public override IReadOnlyList Features { get; } - public override RazorCodeDocument Process(string filePath) + public override RazorCodeDocument Process(RazorProjectItem projectItem) { - if (string.IsNullOrEmpty(filePath)) + if (projectItem == null) { - throw new ArgumentException(Resources.ArgumentCannotBeNullOrEmpty, nameof(filePath)); - } - - var projectItem = FileSystem.GetItem(filePath); - var sourceDocument = RazorSourceDocument.ReadFrom(projectItem); - var codeDocument = Process(sourceDocument); - - return codeDocument; - } - - public override RazorCodeDocument Process(RazorSourceDocument sourceDocument) - { - if (sourceDocument == null) - { - throw new ArgumentNullException(nameof(sourceDocument)); + throw new ArgumentNullException(nameof(projectItem)); } var importFeature = GetRequiredFeature(); - var imports = importFeature.GetImports(sourceDocument.FilePath); + var imports = importFeature.GetImports(projectItem); + var sourceDocument = RazorSourceDocument.ReadFrom(projectItem); var codeDocument = RazorCodeDocument.Create(sourceDocument, imports); diff --git a/src/Microsoft.AspNetCore.Razor.Language/IRazorImportFeature.cs b/src/Microsoft.AspNetCore.Razor.Language/IRazorImportFeature.cs index 76e50c31c7..f002cd20ce 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/IRazorImportFeature.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/IRazorImportFeature.cs @@ -7,6 +7,6 @@ namespace Microsoft.AspNetCore.Razor.Language { public interface IRazorImportFeature : IRazorProjectEngineFeature { - IReadOnlyList GetImports(string sourceFilePath); + IReadOnlyList GetImports(RazorProjectItem projectItem); } } diff --git a/src/Microsoft.AspNetCore.Razor.Language/RazorProjectEngine.cs b/src/Microsoft.AspNetCore.Razor.Language/RazorProjectEngine.cs index 88522e9326..7f2c03e8e3 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/RazorProjectEngine.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/RazorProjectEngine.cs @@ -14,9 +14,7 @@ namespace Microsoft.AspNetCore.Razor.Language public abstract IReadOnlyList Features { get; } - public abstract RazorCodeDocument Process(string filePath); - - public abstract RazorCodeDocument Process(RazorSourceDocument sourceDocument); + public abstract RazorCodeDocument Process(RazorProjectItem projectItem); public static RazorProjectEngine Create(RazorProjectFileSystem fileSystem) => Create(fileSystem, configure: null); diff --git a/src/Microsoft.AspNetCore.Razor.Language/RazorProjectEngineFeatureBase.cs b/src/Microsoft.AspNetCore.Razor.Language/RazorProjectEngineFeatureBase.cs index f5944a83ff..ec170659e4 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/RazorProjectEngineFeatureBase.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/RazorProjectEngineFeatureBase.cs @@ -9,7 +9,7 @@ namespace Microsoft.AspNetCore.Razor.Language { private RazorProjectEngine _projectEngine; - public RazorProjectEngine ProjectEngine + public virtual RazorProjectEngine ProjectEngine { get => _projectEngine; set diff --git a/src/Microsoft.AspNetCore.Razor.Language/VirtualRazorProjectFileSystem.cs b/src/Microsoft.AspNetCore.Razor.Language/VirtualRazorProjectFileSystem.cs index bb9a75ee3a..6b3737fb73 100644 --- a/src/Microsoft.AspNetCore.Razor.Language/VirtualRazorProjectFileSystem.cs +++ b/src/Microsoft.AspNetCore.Razor.Language/VirtualRazorProjectFileSystem.cs @@ -10,20 +10,19 @@ namespace Microsoft.AspNetCore.Razor.Language { internal class VirtualRazorProjectFileSystem : RazorProjectFileSystem { - private readonly DirectoryNode Root = new DirectoryNode("/"); + private readonly DirectoryNode _root = new DirectoryNode("/"); public override IEnumerable EnumerateItems(string basePath) { - basePath = NormalizeAndEnsureValidPath(basePath); - var directory = Root.GetDirectory(basePath); + var directory = _root.GetDirectory(basePath); return directory?.EnumerateItems() ?? Enumerable.Empty(); } public override RazorProjectItem GetItem(string path) { path = NormalizeAndEnsureValidPath(path); - return Root.GetItem(path) ?? new NotFoundProjectItem(string.Empty, path); + return _root.GetItem(path) ?? new NotFoundProjectItem(string.Empty, path); } public void Add(RazorProjectItem projectItem) @@ -34,7 +33,7 @@ namespace Microsoft.AspNetCore.Razor.Language } var filePath = NormalizeAndEnsureValidPath(projectItem.FilePath); - Root.AddFile(new FileNode(filePath, projectItem)); + _root.AddFile(new FileNode(filePath, projectItem)); } // Internal for testing diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/DefaultMvcImportFeatureTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/DefaultMvcImportFeatureTest.cs index 32e254a45b..3ae6d8dd13 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/DefaultMvcImportFeatureTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Test/DefaultMvcImportFeatureTest.cs @@ -29,12 +29,13 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions { // Arrange var imports = new List(); + var projectItem = new TestRazorProjectItem("/Contact/Index.cshtml"); var testFileSystem = new TestRazorProjectFileSystem(new[] { new TestRazorProjectItem("/Index.cshtml"), new TestRazorProjectItem("/_ViewImports.cshtml"), new TestRazorProjectItem("/Contact/_ViewImports.cshtml"), - new TestRazorProjectItem("/Contact/Index.cshtml"), + projectItem, }); var mvcImportFeature = new DefaultMvcImportFeature() { @@ -42,7 +43,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions }; // Act - mvcImportFeature.AddHierarchicalImports("/Contact/Index.cshtml", imports); + mvcImportFeature.AddHierarchicalImports(projectItem, imports); // Assert Assert.Collection(imports, @@ -55,17 +56,15 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions { // Arrange var imports = new List(); - var testFileSystem = new TestRazorProjectFileSystem(new[] - { - new TestRazorProjectItem("/Pages/Contact/Index.cshtml"), - }); + var projectItem = new TestRazorProjectItem("/Pages/Contact/Index.cshtml"); + var testFileSystem = new TestRazorProjectFileSystem(new[] { projectItem }); var mvcImportFeature = new DefaultMvcImportFeature() { ProjectEngine = Mock.Of(projectEngine => projectEngine.FileSystem == testFileSystem) }; // Act - mvcImportFeature.AddHierarchicalImports("/Pages/Contact/Index.cshtml", imports); + mvcImportFeature.AddHierarchicalImports(projectItem, imports); // Assert Assert.Collection(imports, diff --git a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X.Test/DefaultMvcImportFeatureTest.cs b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X.Test/DefaultMvcImportFeatureTest.cs index 31d66517a1..3741760b4a 100644 --- a/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X.Test/DefaultMvcImportFeatureTest.cs +++ b/test/Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X.Test/DefaultMvcImportFeatureTest.cs @@ -29,12 +29,13 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X { // Arrange var imports = new List(); + var projectItem = new TestRazorProjectItem("/Contact/Index.cshtml"); var testFileSystem = new TestRazorProjectFileSystem(new[] { new TestRazorProjectItem("/Index.cshtml"), new TestRazorProjectItem("/_ViewImports.cshtml"), new TestRazorProjectItem("/Contact/_ViewImports.cshtml"), - new TestRazorProjectItem("/Contact/Index.cshtml"), + projectItem, }); var mvcImportFeature = new DefaultMvcImportFeature() { @@ -42,7 +43,7 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X }; // Act - mvcImportFeature.AddHierarchicalImports("/Contact/Index.cshtml", imports); + mvcImportFeature.AddHierarchicalImports(projectItem, imports); // Assert Assert.Collection(imports, @@ -55,17 +56,15 @@ namespace Microsoft.AspNetCore.Mvc.Razor.Extensions.Version1_X { // Arrange var imports = new List(); - var testFileSystem = new TestRazorProjectFileSystem(new[] - { - new TestRazorProjectItem("/Pages/Contact/Index.cshtml"), - }); + var projectItem = new TestRazorProjectItem("/Pages/Contact/Index.cshtml"); + var testFileSystem = new TestRazorProjectFileSystem(new[] { projectItem }); var mvcImportFeature = new DefaultMvcImportFeature() { ProjectEngine = Mock.Of(projectEngine => projectEngine.FileSystem == testFileSystem) }; // Act - mvcImportFeature.AddHierarchicalImports("/Pages/Contact/Index.cshtml", imports); + mvcImportFeature.AddHierarchicalImports(projectItem, imports); // Assert Assert.Collection(imports, diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/DefaultRazorProjectEngineIntegrationTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/DefaultRazorProjectEngineIntegrationTest.cs index ce95f80f6f..e767978411 100644 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/DefaultRazorProjectEngineIntegrationTest.cs +++ b/test/Microsoft.AspNetCore.Razor.Language.Test/DefaultRazorProjectEngineIntegrationTest.cs @@ -12,10 +12,10 @@ namespace Microsoft.AspNetCore.Razor.Language public void Process_GetsImportsFromFeature() { // Arrange - var sourceDocument = TestRazorSourceDocument.Create(); + var projectItem = new TestRazorProjectItem("Index.cshtml"); var testImport = TestRazorSourceDocument.Create(); var importFeature = new Mock(); - importFeature.Setup(feature => feature.GetImports(It.IsAny())) + importFeature.Setup(feature => feature.GetImports(It.IsAny())) .Returns(new[] { testImport }); var projectEngine = RazorProjectEngine.Create(TestRazorProjectFileSystem.Empty, builder => { @@ -23,7 +23,7 @@ namespace Microsoft.AspNetCore.Razor.Language }); // Act - var codeDocument = projectEngine.Process(sourceDocument); + var codeDocument = projectEngine.Process(projectItem); // Assert var import = Assert.Single(codeDocument.Imports); @@ -34,11 +34,11 @@ namespace Microsoft.AspNetCore.Razor.Language public void Process_GeneratesCodeDocumentWithValidCSharpDocument() { // Arrange - var sourceDocument = TestRazorSourceDocument.Create(); + var projectItem = new TestRazorProjectItem("Index.cshtml"); var projectEngine = RazorProjectEngine.Create(TestRazorProjectFileSystem.Empty); // Act - var codeDocument = projectEngine.Process(sourceDocument); + var codeDocument = projectEngine.Process(projectItem); // Assert var csharpDocument = codeDocument.GetCSharpDocument(); diff --git a/test/Microsoft.AspNetCore.Razor.Language.Test/DefaultRazorProjectEngineTest.cs b/test/Microsoft.AspNetCore.Razor.Language.Test/DefaultRazorProjectEngineTest.cs deleted file mode 100644 index 3224a5d9f0..0000000000 --- a/test/Microsoft.AspNetCore.Razor.Language.Test/DefaultRazorProjectEngineTest.cs +++ /dev/null @@ -1,29 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Moq; -using Xunit; - -namespace Microsoft.AspNetCore.Razor.Language -{ - public class DefaultRazorProjectEngineTest - { - [Fact] - public void Process_AsksFileSystemForItems() - { - // Arrange - var razorProjectItem = new TestRazorProjectItem("/some/path.cshtml"); - var testFileSystem = new Mock(); - testFileSystem.Setup(fileSystem => fileSystem.GetItem("/some/path.cshtml")) - .Returns(razorProjectItem) - .Verifiable(); - var projectEngine = RazorProjectEngine.Create(testFileSystem.Object); - - // Act - projectEngine.Process("/some/path.cshtml"); - - // Assert - testFileSystem.Verify(); - } - } -}