From 83187945d1e72b8d632aeb14e4127a3b3af2381f Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 23 Oct 2014 14:33:54 -0700 Subject: [PATCH] Fix for #1052 - ViewComponents should support fully qualified names This change adds the concept of a full-name to viewcomponents. View components can be invoked using either the short name or long name. If the provided string contains a '.' character, then it will be compared against full names, otherwise it will be matched against short names only. The short name is used for view lookups. If the name is explicitly set via ViewComponent attribute, then the full name is the name provided. The short name is the portion of the name after the last '.'. If there are no dots, then the short name and full name are the same. If the name is not set explicitly, then it is inferred from the Type and namespace name. The short name is the Type name, minus the 'ViewComponent' suffix (if present). The full name is the namespace of the defining class, plus the short name. --- .../Properties/Resources.Designer.cs | 24 ++- src/Microsoft.AspNet.Mvc.Core/Resources.resx | 6 +- .../DefaultViewComponentSelector.cs | 79 ++++++- .../ViewComponentConventions.cs | 34 +++ .../DefaultViewComponentSelectorTest.cs | 193 ++++++++++++++++++ .../ViewComponentTests.cs | 30 +++ .../FullNameController.cs | 16 ++ .../IntegerViewComponent.cs | 5 + .../Namespace1/SameNameViewComponent.cs | 16 ++ .../Namespace2/SameNameViewComponent.cs | 16 ++ .../Views/FullName/Invoke.cshtml | 1 + 11 files changed, 404 insertions(+), 16 deletions(-) create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/ViewComponents/DefaultViewComponentSelectorTest.cs create mode 100644 test/WebSites/ViewComponentWebSite/FullNameController.cs create mode 100644 test/WebSites/ViewComponentWebSite/Namespace1/SameNameViewComponent.cs create mode 100644 test/WebSites/ViewComponentWebSite/Namespace2/SameNameViewComponent.cs create mode 100644 test/WebSites/ViewComponentWebSite/Views/FullName/Invoke.cshtml diff --git a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs index ebeb1fbcea..bb9de0d044 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Properties/Resources.Designer.cs @@ -251,7 +251,7 @@ namespace Microsoft.AspNet.Mvc.Core } /// - /// The view component name '{0}' matched multiple types: {1} + /// The view component name '{0}' matched multiple types:{1}{2} /// internal static string ViewComponent_AmbiguousTypeMatch { @@ -259,11 +259,11 @@ namespace Microsoft.AspNet.Mvc.Core } /// - /// The view component name '{0}' matched multiple types: {1} + /// The view component name '{0}' matched multiple types:{1}{2} /// - internal static string FormatViewComponent_AmbiguousTypeMatch(object p0, object p1) + internal static string FormatViewComponent_AmbiguousTypeMatch(object p0, object p1, object p2) { - return string.Format(CultureInfo.CurrentCulture, GetString("ViewComponent_AmbiguousTypeMatch"), p0, p1); + return string.Format(CultureInfo.CurrentCulture, GetString("ViewComponent_AmbiguousTypeMatch"), p0, p1, p2); } /// @@ -1530,6 +1530,22 @@ namespace Microsoft.AspNet.Mvc.Core return string.Format(CultureInfo.CurrentCulture, GetString("FileResult_InvalidPath"), p0); } + /// + /// Type: '{0}' - Name: '{1}' + /// + internal static string ViewComponent_AmbiguousTypeMatch_Item + { + get { return GetString("ViewComponent_AmbiguousTypeMatch_Item"); } + } + + /// + /// Type: '{0}' - Name: '{1}' + /// + internal static string FormatViewComponent_AmbiguousTypeMatch_Item(object p0, object p1) + { + return string.Format(CultureInfo.CurrentCulture, GetString("ViewComponent_AmbiguousTypeMatch_Item"), p0, p1); + } + private static string GetString(string name, params string[] formatterNames) { var value = _resourceManager.GetString(name); diff --git a/src/Microsoft.AspNet.Mvc.Core/Resources.resx b/src/Microsoft.AspNet.Mvc.Core/Resources.resx index 9f9b9be8e3..4714a8d58e 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Resources.resx +++ b/src/Microsoft.AspNet.Mvc.Core/Resources.resx @@ -163,7 +163,8 @@ The class ReflectedActionFilterEndPoint only supports ReflectedActionDescriptors. - The view component name '{0}' matched multiple types: {1} + The view component name '{0}' matched multiple types:{1}{2} + {1} is the newline character The async view component method '{0}' should be declared to return Task<T>. @@ -413,4 +414,7 @@ Could not find file: {0} {0} is the value for the provided path + + Type: '{0}' - Name: '{1}' + \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentSelector.cs b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentSelector.cs index 87338f5bec..c19c868d2d 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentSelector.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/DefaultViewComponentSelector.cs @@ -2,7 +2,10 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; +using System.Diagnostics.Contracts; using System.Linq; +using System.Reflection; using Microsoft.AspNet.Mvc.Core; namespace Microsoft.AspNet.Mvc @@ -21,30 +24,84 @@ namespace Microsoft.AspNet.Mvc var assemblies = _assemblyProvider.CandidateAssemblies; var types = assemblies.SelectMany(a => a.DefinedTypes); - var components = + var candidates = types - .Where(ViewComponentConventions.IsComponent) - .Select(c => new { Name = ViewComponentConventions.GetComponentName(c), Type = c.AsType() }); + .Where(t => IsViewComponentType(t)) + .Select(CreateCandidate); - var matching = - components - .Where(c => string.Equals(c.Name, componentName, StringComparison.OrdinalIgnoreCase)) - .ToArray(); + // ViewComponent names can either be fully-qualified, or refer to the 'short-name'. If the provided + // name contains a '.' - then it's a fully-qualified name. + var matching = new List(); + if (componentName.Contains(".")) + { + matching.AddRange(candidates.Where( + c => string.Equals(c.FullName, componentName, StringComparison.OrdinalIgnoreCase))); + } + else + { + matching.AddRange(candidates.Where( + c => string.Equals(c.ShortName, componentName, StringComparison.OrdinalIgnoreCase))); + } - if (matching.Length == 0) + if (matching.Count == 0) { return null; } - else if (matching.Length == 1) + else if (matching.Count == 1) { return matching[0].Type; } else { - var typeNames = string.Join(Environment.NewLine, matching.Select(t => t.Type.FullName)); + var matchedTypes = new List(); + foreach (var candidate in matching) + { + matchedTypes.Add(Resources.FormatViewComponent_AmbiguousTypeMatch_Item( + candidate.Type.FullName, + candidate.FullName)); + } + + var typeNames = string.Join(Environment.NewLine, matchedTypes); throw new InvalidOperationException( - Resources.FormatViewComponent_AmbiguousTypeMatch(componentName, typeNames)); + Resources.FormatViewComponent_AmbiguousTypeMatch(componentName, Environment.NewLine, typeNames)); } } + + protected virtual bool IsViewComponentType([NotNull] TypeInfo typeInfo) + { + return ViewComponentConventions.IsComponent(typeInfo); + } + + private static ViewComponentCandidate CreateCandidate(TypeInfo typeInfo) + { + var candidate = new ViewComponentCandidate() + { + FullName = ViewComponentConventions.GetComponentFullName(typeInfo), + ShortName = ViewComponentConventions.GetComponentName(typeInfo), + Type = typeInfo.AsType(), + }; + + Contract.Assert(!string.IsNullOrEmpty(candidate.FullName)); + var separatorIndex = candidate.FullName.LastIndexOf("."); + if (separatorIndex >= 0) + { + candidate.ShortName = candidate.FullName.Substring(separatorIndex + 1); + } + else + { + candidate.ShortName = candidate.FullName; + } + + return candidate; + } + + private class ViewComponentCandidate + { + public string FullName { get; set; } + + public string ShortName { get; set; } + + public Type Type { get; set; } + } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponentConventions.cs b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponentConventions.cs index 1fa23c8cfe..bb913f0b00 100644 --- a/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponentConventions.cs +++ b/src/Microsoft.AspNet.Mvc.Core/ViewComponents/ViewComponentConventions.cs @@ -14,10 +14,44 @@ namespace Microsoft.AspNet.Mvc { var attribute = componentType.GetCustomAttribute(); if (attribute != null && !string.IsNullOrEmpty(attribute.Name)) + { + var separatorIndex = attribute.Name.LastIndexOf('.'); + if (separatorIndex >= 0) + { + return attribute.Name.Substring(separatorIndex + 1); + } + else + { + return attribute.Name; + } + } + + return GetShortNameByConvention(componentType); + } + + public static string GetComponentFullName([NotNull] TypeInfo componentType) + { + var attribute = componentType.GetCustomAttribute(); + if (!string.IsNullOrEmpty(attribute?.Name)) { return attribute.Name; } + // If the view component didn't define a name explicitly then use the namespace + the + // 'short name'. + var shortName = GetShortNameByConvention(componentType); + if (string.IsNullOrEmpty(componentType.Namespace)) + { + return shortName; + } + else + { + return componentType.Namespace + "." + shortName; + } + } + + private static string GetShortNameByConvention(TypeInfo componentType) + { if (componentType.Name.EndsWith(ViewComponentSuffix, StringComparison.OrdinalIgnoreCase)) { return componentType.Name.Substring(0, componentType.Name.Length - ViewComponentSuffix.Length); diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/ViewComponents/DefaultViewComponentSelectorTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/ViewComponents/DefaultViewComponentSelectorTest.cs new file mode 100644 index 0000000000..7a37abf641 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/ViewComponents/DefaultViewComponentSelectorTest.cs @@ -0,0 +1,193 @@ +// 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.Linq; +using System.Reflection; +using Xunit; + +namespace Microsoft.AspNet.Mvc +{ + public class DefaultViewComponentSelectorTest + { + [Fact] + public void SelectComponent_ByShortNameWithSuffix() + { + // Arrange + var selector = CreateSelector(); + + // Act + var result = selector.SelectComponent("Suffix"); + + // Assert + Assert.Equal(typeof(SuffixViewComponent), result); + } + + [Fact] + public void SelectComponent_ByLongNameWithSuffix() + { + // Arrange + var selector = CreateSelector(); + + // Act + var result = selector.SelectComponent("Microsoft.AspNet.Mvc.Suffix"); + + // Assert + Assert.Equal(typeof(SuffixViewComponent), result); + } + + [Fact] + public void SelectComponent_ByShortNameWithoutSuffix() + { + // Arrange + var selector = CreateSelector(); + + // Act + var result = selector.SelectComponent("WithoutSuffix"); + + // Assert + Assert.Equal(typeof(WithoutSuffix), result); + } + + [Fact] + public void SelectComponent_ByLongNameWithoutSuffix() + { + // Arrange + var selector = CreateSelector(); + + // Act + var result = selector.SelectComponent("Microsoft.AspNet.Mvc.WithoutSuffix"); + + // Assert + Assert.Equal(typeof(WithoutSuffix), result); + } + + [Fact] + public void SelectComponent_ByAttribute() + { + // Arrange + var selector = CreateSelector(); + + // Act + var result = selector.SelectComponent("ByAttribute"); + + // Assert + Assert.Equal(typeof(ByAttribute), result); + } + + [Fact] + public void SelectComponent_ByNamingConvention() + { + // Arrange + var selector = CreateSelector(); + + // Act + var result = selector.SelectComponent("ByNamingConvention"); + + // Assert + Assert.Equal(typeof(ByNamingConventionViewComponent), result); + } + + [Fact] + public void SelectComponent_Ambiguity() + { + // Arrange + var selector = CreateSelector(); + + var expected = + "The view component name 'Ambiguous' matched multiple types:" + Environment.NewLine + + "Type: 'Microsoft.AspNet.Mvc.DefaultViewComponentSelectorTest+Ambiguous1' - " + + "Name: 'Namespace1.Ambiguous'" + Environment.NewLine + + "Type: 'Microsoft.AspNet.Mvc.DefaultViewComponentSelectorTest+Ambiguous2' - " + + "Name: 'Namespace2.Ambiguous'"; + + // Act + var ex = Assert.Throws(() => selector.SelectComponent("Ambiguous")); + + // Assert + Assert.Equal(expected, ex.Message); + } + + [Fact] + public void SelectComponent_FullNameToAvoidAmbiguity() + { + // Arrange + var selector = CreateSelector(); + + // Act + var result = selector.SelectComponent("Namespace1.Ambiguous"); + + // Assert + Assert.Equal(typeof(Ambiguous1), result); + } + + [Theory] + [InlineData("FullNameInAttribute")] + [InlineData("CoolNameSpace.FullNameInAttribute")] + public void SelectComponent_FullNameInAttribute(string name) + { + // Arrange + var selector = CreateSelector(); + + // Act + var result = selector.SelectComponent(name); + + // Assert + Assert.Equal(typeof(FullNameInAttribute), result); + } + + private IViewComponentSelector CreateSelector() + { + return new FilteredViewComponentSelector(); + } + + private class SuffixViewComponent : ViewComponent + { + } + + private class WithoutSuffix : ViewComponent + { + } + + private class ByNamingConventionViewComponent + { + } + + [ViewComponent] + private class ByAttribute + { + } + + [ViewComponent(Name = "Namespace1.Ambiguous")] + private class Ambiguous1 + { + } + + [ViewComponent(Name = "Namespace2.Ambiguous")] + private class Ambiguous2 + { + } + + [ViewComponent(Name = "CoolNameSpace.FullNameInAttribute")] + private class FullNameInAttribute + { + } + + // This will only consider types nested inside this class as ViewComponent classes + private class FilteredViewComponentSelector : DefaultViewComponentSelector + { + public FilteredViewComponentSelector() + : base(new StaticAssemblyProvider()) + { + AllowedTypes = typeof(DefaultViewComponentSelectorTest).GetNestedTypes(BindingFlags.NonPublic); + } + + public Type[] AllowedTypes { get; private set; } + + protected override bool IsViewComponentType([NotNull] TypeInfo typeInfo) + { + return AllowedTypes.Contains(typeInfo.AsType()); + } + } + } +} \ No newline at end of file diff --git a/test/Microsoft.AspNet.Mvc.FunctionalTests/ViewComponentTests.cs b/test/Microsoft.AspNet.Mvc.FunctionalTests/ViewComponentTests.cs index dcd94a11fe..1080a64b02 100644 --- a/test/Microsoft.AspNet.Mvc.FunctionalTests/ViewComponentTests.cs +++ b/test/Microsoft.AspNet.Mvc.FunctionalTests/ViewComponentTests.cs @@ -64,5 +64,35 @@ namespace Microsoft.AspNet.Mvc.FunctionalTests // Assert Assert.Equal("10", body.Trim()); } + + [Theory] + [InlineData("ViewComponentWebSite.Namespace1.SameName")] + [InlineData("ViewComponentWebSite.Namespace2.SameName")] + public async Task ViewComponents_FullName(string name) + { + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + // Act + var body = await client.GetStringAsync("http://localhost/FullName/Invoke?name=" + name); + + // Assert + Assert.Equal(name, body.Trim()); + } + + [Fact] + public async Task ViewComponents_ShortNameUsedForViewLookup() + { + var server = TestServer.Create(_provider, _app); + var client = server.CreateClient(); + + var name = "ViewComponentWebSite.Integer"; + + // Act + var body = await client.GetStringAsync("http://localhost/FullName/Invoke?name=" + name); + + // Assert + Assert.Equal("17", body.Trim()); + } } } diff --git a/test/WebSites/ViewComponentWebSite/FullNameController.cs b/test/WebSites/ViewComponentWebSite/FullNameController.cs new file mode 100644 index 0000000000..b78173b624 --- /dev/null +++ b/test/WebSites/ViewComponentWebSite/FullNameController.cs @@ -0,0 +1,16 @@ +// 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 Microsoft.AspNet.Mvc; + +namespace ViewComponentWebSite +{ + public class FullNameController : Controller + { + public IActionResult Invoke(string name) + { + ViewBag.Name = name; + return View(); + } + } +} \ No newline at end of file diff --git a/test/WebSites/ViewComponentWebSite/IntegerViewComponent.cs b/test/WebSites/ViewComponentWebSite/IntegerViewComponent.cs index d44ffb4349..b08786884c 100644 --- a/test/WebSites/ViewComponentWebSite/IntegerViewComponent.cs +++ b/test/WebSites/ViewComponentWebSite/IntegerViewComponent.cs @@ -7,6 +7,11 @@ namespace ViewComponentWebSite { public class IntegerViewComponent : ViewComponent { + public IViewComponentResult Invoke() + { + return Invoke(17); + } + public IViewComponentResult Invoke(int valueFromView) { return View(valueFromView); diff --git a/test/WebSites/ViewComponentWebSite/Namespace1/SameNameViewComponent.cs b/test/WebSites/ViewComponentWebSite/Namespace1/SameNameViewComponent.cs new file mode 100644 index 0000000000..c044fa5981 --- /dev/null +++ b/test/WebSites/ViewComponentWebSite/Namespace1/SameNameViewComponent.cs @@ -0,0 +1,16 @@ +// 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 Microsoft.AspNet.Mvc; + +namespace ViewComponentWebSite.Namespace1 +{ + // The full name is different here from the other view component with the same short name. + public class SameNameViewComponent : ViewComponent + { + public string Invoke() + { + return "ViewComponentWebSite.Namespace1.SameName"; + } + } +} \ No newline at end of file diff --git a/test/WebSites/ViewComponentWebSite/Namespace2/SameNameViewComponent.cs b/test/WebSites/ViewComponentWebSite/Namespace2/SameNameViewComponent.cs new file mode 100644 index 0000000000..1d40437390 --- /dev/null +++ b/test/WebSites/ViewComponentWebSite/Namespace2/SameNameViewComponent.cs @@ -0,0 +1,16 @@ +// 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 Microsoft.AspNet.Mvc; + +namespace ViewComponentWebSite.Namespace2 +{ + // The full name is different here from the other view component with the same short name. + public class SameNameViewComponent : ViewComponent + { + public string Invoke() + { + return "ViewComponentWebSite.Namespace2.SameName"; + } + } +} \ No newline at end of file diff --git a/test/WebSites/ViewComponentWebSite/Views/FullName/Invoke.cshtml b/test/WebSites/ViewComponentWebSite/Views/FullName/Invoke.cshtml new file mode 100644 index 0000000000..f87bef07aa --- /dev/null +++ b/test/WebSites/ViewComponentWebSite/Views/FullName/Invoke.cshtml @@ -0,0 +1 @@ +@Component.Invoke(ViewBag.Name) \ No newline at end of file