From 8ea196023e8909b3d731fa607f89af3791f74c62 Mon Sep 17 00:00:00 2001 From: Pranav K Date: Thu, 13 Mar 2014 17:55:53 -0700 Subject: [PATCH] Modify controller discovery to only look for types that reference Mvc.Core assembly --- .../AppDomainControllerAssemblyProvider.cs | 30 --------- .../DefaultControllerAssemblyProvider.cs | 57 ++++++++++++++++ .../IControllerAssemblyProvider.cs | 2 +- .../Properties/AssemblyInfo.cs | 1 + .../ReflectedActionDescriptorProvider.cs | 2 +- .../Services/AssemblyNeutralAttribute.cs | 11 ++++ .../Services/ILibraryExport.cs | 12 ++++ .../Services/ILibraryInformation.cs | 12 ++++ .../Services/ILibraryManager.cs | 16 +++++ .../Services/IMetadataReference.cs | 9 +++ .../Services/ISourceReference.cs | 7 ++ .../Compilation/RoslynCompilationService.cs | 8 +-- .../Services/ILibraryExportProvider.cs | 10 --- .../Services/ILibraryInformation.cs | 12 ++++ .../Services/ILibraryManager.cs | 16 +++++ src/Microsoft.AspNet.Mvc/MvcServices.cs | 2 +- .../DefaultControllerAssemblyProviderTests.cs | 65 +++++++++++++++++++ 17 files changed, 225 insertions(+), 47 deletions(-) delete mode 100644 src/Microsoft.AspNet.Mvc.Core/AppDomainControllerAssemblyProvider.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/DefaultControllerAssemblyProvider.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Services/AssemblyNeutralAttribute.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Services/ILibraryExport.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Services/ILibraryInformation.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Services/ILibraryManager.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Services/IMetadataReference.cs create mode 100644 src/Microsoft.AspNet.Mvc.Core/Services/ISourceReference.cs delete mode 100644 src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryExportProvider.cs create mode 100644 src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryInformation.cs create mode 100644 src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryManager.cs create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerAssemblyProviderTests.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/AppDomainControllerAssemblyProvider.cs b/src/Microsoft.AspNet.Mvc.Core/AppDomainControllerAssemblyProvider.cs deleted file mode 100644 index a676e0f743..0000000000 --- a/src/Microsoft.AspNet.Mvc.Core/AppDomainControllerAssemblyProvider.cs +++ /dev/null @@ -1,30 +0,0 @@ -using System; -using System.Collections.Generic; -using System.Linq; -using System.Reflection; - -namespace Microsoft.AspNet.Mvc -{ - public class AppDomainControllerAssemblyProvider : IControllerAssemblyProvider - { - public IEnumerable Assemblies - { - get - { - foreach (var assembly in AppDomain.CurrentDomain.GetAssemblies().Where(AllowAssembly)) - { - yield return assembly; - } - } - } - - private bool AllowAssembly(Assembly assembly) - { - // consider mechanisms to filter assemblies upfront, so scanning cost is minimized and startup improved. - // 1 - Does assembly reference the WebFx assembly (directly or indirectly). - Down side, object only controller not supported. - // 2 - Remove well known assemblies (maintenance and composability cost) - - return true; - } - } -} diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultControllerAssemblyProvider.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerAssemblyProvider.cs new file mode 100644 index 0000000000..814c96643c --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultControllerAssemblyProvider.cs @@ -0,0 +1,57 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using Microsoft.Net.Runtime; + +namespace Microsoft.AspNet.Mvc +{ + public class DefaultControllerAssemblyProvider : IControllerAssemblyProvider + { + // List of Mvc assemblies that we'll use as roots for controller discovery. + private static readonly HashSet _mvcAssemblyList = new HashSet(StringComparer.Ordinal) + { + "Microsoft.AspNet.Mvc", + "Microsoft.AspNet.Mvc.Core", + "Microsoft.AspNet.Mvc.ModelBinding", + "Microsoft.AspNet.Mvc.Razor", + "Microsoft.AspNet.Mvc.Razor.Host", + "Microsoft.AspNet.Mvc.Rendering", + }; + private readonly ILibraryManager _libraryManager; + private readonly Func _assemblyLoader; + + public DefaultControllerAssemblyProvider(ILibraryManager libraryManager) + { + _libraryManager = libraryManager; + } + + public IEnumerable CandidateAssemblies + { + get + { + return GetCandiateLibraries().Select(Load); + } + } + + internal IEnumerable GetCandiateLibraries() + { + // GetReferencingLibraries returns the transitive closure of referencing assemblies + // for a given assembly. In our case, we'll gather all assemblies that reference + // any of the primary Mvc assemblies while ignoring Mvc assemblies. + return _mvcAssemblyList.SelectMany(_libraryManager.GetReferencingLibraries) + .Distinct() + .Where(IsCandidateLibrary); + } + + private static Assembly Load(ILibraryInformation library) + { + return Assembly.Load(new AssemblyName(library.Name)); + } + + private static bool IsCandidateLibrary(ILibraryInformation library) + { + return !_mvcAssemblyList.Contains(library.Name); + } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/IControllerAssemblyProvider.cs b/src/Microsoft.AspNet.Mvc.Core/IControllerAssemblyProvider.cs index 131be36eda..d9fa723c4f 100644 --- a/src/Microsoft.AspNet.Mvc.Core/IControllerAssemblyProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/IControllerAssemblyProvider.cs @@ -5,6 +5,6 @@ namespace Microsoft.AspNet.Mvc { public interface IControllerAssemblyProvider { - IEnumerable Assemblies { get; } + IEnumerable CandidateAssemblies { get; } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/AssemblyInfo.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/AssemblyInfo.cs index 301a9d302b..5392ca83bd 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/AssemblyInfo.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/AssemblyInfo.cs @@ -34,3 +34,4 @@ using System.Runtime.InteropServices; // [assembly: AssemblyVersion("1.0.*")] [assembly: AssemblyVersion("1.0.0.0")] [assembly: AssemblyFileVersion("1.0.0.0")] +[assembly: InternalsVisibleTo("Microsoft.AspNet.Mvc.Core.Test")] diff --git a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs index aa6729bf35..7b7d04d1d4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ReflectedActionDescriptorProvider.cs @@ -43,7 +43,7 @@ namespace Microsoft.AspNet.Mvc public IEnumerable GetDescriptors() { - var assemblies = _controllerAssemblyProvider.Assemblies; + var assemblies = _controllerAssemblyProvider.CandidateAssemblies; var types = assemblies.SelectMany(a => a.DefinedTypes); var controllers = types.Where(_conventions.IsController); var controllerDescriptors = controllers.Select(t => _controllerDescriptorFactory.CreateControllerDescriptor(t)).ToArray(); diff --git a/src/Microsoft.AspNet.Mvc.Core/Services/AssemblyNeutralAttribute.cs b/src/Microsoft.AspNet.Mvc.Core/Services/AssemblyNeutralAttribute.cs new file mode 100644 index 0000000000..c0d5952411 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Services/AssemblyNeutralAttribute.cs @@ -0,0 +1,11 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Text; +using System.Threading.Tasks; + +namespace Microsoft.Net.Runtime +{ + [AssemblyNeutral] + public class AssemblyNeutralAttribute : Attribute { } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Services/ILibraryExport.cs b/src/Microsoft.AspNet.Mvc.Core/Services/ILibraryExport.cs new file mode 100644 index 0000000000..7ccb5fcc67 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Services/ILibraryExport.cs @@ -0,0 +1,12 @@ +using System; +using System.Collections.Generic; + +namespace Microsoft.Net.Runtime +{ + [AssemblyNeutral] + public interface ILibraryExport + { + IList MetadataReferences { get; } + IList SourceReferences { get; } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Services/ILibraryInformation.cs b/src/Microsoft.AspNet.Mvc.Core/Services/ILibraryInformation.cs new file mode 100644 index 0000000000..9d47458cf5 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Services/ILibraryInformation.cs @@ -0,0 +1,12 @@ +using System.Collections.Generic; + +namespace Microsoft.Net.Runtime +{ + [AssemblyNeutral] + public interface ILibraryInformation + { + string Name { get; } + + IEnumerable Dependencies { get; } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Services/ILibraryManager.cs b/src/Microsoft.AspNet.Mvc.Core/Services/ILibraryManager.cs new file mode 100644 index 0000000000..1f714af5d9 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Services/ILibraryManager.cs @@ -0,0 +1,16 @@ +using System.Collections.Generic; + +namespace Microsoft.Net.Runtime +{ + [AssemblyNeutral] + public interface ILibraryManager + { + ILibraryExport GetLibraryExport(string name); + + IEnumerable GetReferencingLibraries(string name); + + ILibraryInformation GetLibraryInformation(string name); + + IEnumerable GetLibraries(); + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Services/IMetadataReference.cs b/src/Microsoft.AspNet.Mvc.Core/Services/IMetadataReference.cs new file mode 100644 index 0000000000..851746ca4f --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Services/IMetadataReference.cs @@ -0,0 +1,9 @@ + +namespace Microsoft.Net.Runtime +{ + [AssemblyNeutral] + public interface IMetadataReference + { + string Name { get; } + } +} diff --git a/src/Microsoft.AspNet.Mvc.Core/Services/ISourceReference.cs b/src/Microsoft.AspNet.Mvc.Core/Services/ISourceReference.cs new file mode 100644 index 0000000000..cfb404e908 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Services/ISourceReference.cs @@ -0,0 +1,7 @@ +namespace Microsoft.Net.Runtime +{ + [AssemblyNeutral] + public interface ISourceReference + { + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Razor/Compilation/RoslynCompilationService.cs b/src/Microsoft.AspNet.Mvc.Razor/Compilation/RoslynCompilationService.cs index 412e3ff4d3..4c34b968e3 100644 --- a/src/Microsoft.AspNet.Mvc.Razor/Compilation/RoslynCompilationService.cs +++ b/src/Microsoft.AspNet.Mvc.Razor/Compilation/RoslynCompilationService.cs @@ -12,17 +12,17 @@ namespace Microsoft.AspNet.Mvc.Razor.Compilation { public class RoslynCompilationService : ICompilationService { - private readonly ILibraryExportProvider _exportProvider; + private readonly ILibraryManager _libraryManager; private readonly IApplicationEnvironment _environment; private readonly IAssemblyLoaderEngine _loader; public RoslynCompilationService(IApplicationEnvironment environment, IAssemblyLoaderEngine loaderEngine, - ILibraryExportProvider exportProvider) + ILibraryManager libraryManager) { _environment = environment; _loader = loaderEngine; - _exportProvider = exportProvider; + _libraryManager = libraryManager; } public Task Compile(string content) @@ -67,7 +67,7 @@ namespace Microsoft.AspNet.Mvc.Razor.Compilation { var references = new List(); - var export = _exportProvider.GetLibraryExport(_environment.ApplicationName, _environment.TargetFramework); + var export = _libraryManager.GetLibraryExport(_environment.ApplicationName); foreach (var metadataReference in export.MetadataReferences) { diff --git a/src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryExportProvider.cs b/src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryExportProvider.cs deleted file mode 100644 index 2fc5bcb76e..0000000000 --- a/src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryExportProvider.cs +++ /dev/null @@ -1,10 +0,0 @@ -using System.Runtime.Versioning; - -namespace Microsoft.Net.Runtime -{ - [AssemblyNeutral] - public interface ILibraryExportProvider - { - ILibraryExport GetLibraryExport(string name, FrameworkName targetFramework); - } -} diff --git a/src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryInformation.cs b/src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryInformation.cs new file mode 100644 index 0000000000..9d47458cf5 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryInformation.cs @@ -0,0 +1,12 @@ +using System.Collections.Generic; + +namespace Microsoft.Net.Runtime +{ + [AssemblyNeutral] + public interface ILibraryInformation + { + string Name { get; } + + IEnumerable Dependencies { get; } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryManager.cs b/src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryManager.cs new file mode 100644 index 0000000000..1f714af5d9 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Razor/Services/ILibraryManager.cs @@ -0,0 +1,16 @@ +using System.Collections.Generic; + +namespace Microsoft.Net.Runtime +{ + [AssemblyNeutral] + public interface ILibraryManager + { + ILibraryExport GetLibraryExport(string name); + + IEnumerable GetReferencingLibraries(string name); + + ILibraryInformation GetLibraryInformation(string name); + + IEnumerable GetLibraries(); + } +} diff --git a/src/Microsoft.AspNet.Mvc/MvcServices.cs b/src/Microsoft.AspNet.Mvc/MvcServices.cs index 688a4a0715..c994a589a3 100644 --- a/src/Microsoft.AspNet.Mvc/MvcServices.cs +++ b/src/Microsoft.AspNet.Mvc/MvcServices.cs @@ -28,7 +28,7 @@ namespace Microsoft.AspNet.Mvc yield return describe.Transient(); yield return describe.Transient(); yield return describe.Transient(); - yield return describe.Transient(); + yield return describe.Transient(); yield return describe.Transient(); yield return describe.Instance(new MvcRazorHost(typeof(RazorView).FullName)); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerAssemblyProviderTests.cs b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerAssemblyProviderTests.cs new file mode 100644 index 0000000000..992b6ee12f --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultControllerAssemblyProviderTests.cs @@ -0,0 +1,65 @@ +using System; +using System.Collections.Generic; +using System.Linq; +using System.Reflection; +using Microsoft.Net.Runtime; +using Moq; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Core +{ + public class DefaultControllerAssemblyProviderTests + { + [Fact] + public void CandidateAssemblies_IgnoresMvcAssemblies() + { + // Arrange + var manager = new Mock(); + manager.Setup(f => f.GetReferencingLibraries(It.IsAny())) + .Returns(new[] + { + CreateLibraryInfo("Microsoft.AspNet.Mvc.Core"), + CreateLibraryInfo("Microsoft.AspNet.Mvc"), + CreateLibraryInfo("Microsoft.AspNet.Mvc.ModelBinding"), + CreateLibraryInfo("SomeRandomAssembly"), + }) + .Verifiable(); + var provider = new DefaultControllerAssemblyProvider(manager.Object); + + // Act + var candidates = provider.GetCandiateLibraries(); + + // Assert + Assert.Equal(new[] { "SomeRandomAssembly" }, candidates.Select(a => a.Name)); + } + + [Fact] + public void CandidateAssemblies_ReturnsLibrariesReferencingAnyMvcAssembly() + { + // Arrange + var manager = new Mock(); + manager.Setup(f => f.GetReferencingLibraries(It.IsAny())) + .Returns(Enumerable.Empty()); + manager.Setup(f => f.GetReferencingLibraries("Microsoft.AspNet.Mvc.Core")) + .Returns(new[] { CreateLibraryInfo("Foo") }); + manager.Setup(f => f.GetReferencingLibraries("Microsoft.AspNet.Mvc.ModelBinding")) + .Returns(new[] { CreateLibraryInfo("Bar") }); + manager.Setup(f => f.GetReferencingLibraries("Microsoft.AspNet.Mvc")) + .Returns(new[] { CreateLibraryInfo("Baz") }); + var provider = new DefaultControllerAssemblyProvider(manager.Object); + + // Act + var candidates = provider.GetCandiateLibraries(); + + // Assert + Assert.Equal(new[] { "Baz", "Foo", "Bar" }, candidates.Select(a => a.Name)); + } + + private static ILibraryInformation CreateLibraryInfo(string name) + { + var info = new Mock(); + info.SetupGet(b => b.Name).Returns(name); + return info.Object; + } + } +} \ No newline at end of file