From d495ecfb24e6390fc72b21dc6aa574f08d94afff Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 14 May 2019 09:34:09 -0700 Subject: [PATCH 1/2] Make RouteAttribute non-inherited Fixes: #5529 Inheriting and looking for inherited route attributes will cause nothing but trouble. We had a bug tracking what to do about this and we decided to make it really clear that routes are not inherited. Previously the attribute was marked as inherited, but we woulnd't look for inherited routes. --- .../Microsoft.AspNetCore.Components.netstandard2.0.cs | 2 +- src/Components/Components/src/RouteAttribute.cs | 4 ++-- src/Components/Components/src/Routing/RouteTable.cs | 9 +++++++-- 3 files changed, 10 insertions(+), 5 deletions(-) diff --git a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs index b615df9df1a9..1500126c495a 100644 --- a/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs +++ b/src/Components/Components/ref/Microsoft.AspNetCore.Components.netstandard2.0.cs @@ -398,7 +398,7 @@ public readonly partial struct RenderHandle public System.Threading.Tasks.Task InvokeAsync(System.Func workItem) { throw null; } public void Render(Microsoft.AspNetCore.Components.RenderFragment renderFragment) { } } - [System.AttributeUsageAttribute(System.AttributeTargets.Class, AllowMultiple=true, Inherited=true)] + [System.AttributeUsageAttribute(System.AttributeTargets.Class, AllowMultiple=true, Inherited=false)] public partial class RouteAttribute : System.Attribute { public RouteAttribute(string template) { } diff --git a/src/Components/Components/src/RouteAttribute.cs b/src/Components/Components/src/RouteAttribute.cs index f46d04910fa7..a503dab19554 100644 --- a/src/Components/Components/src/RouteAttribute.cs +++ b/src/Components/Components/src/RouteAttribute.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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 System; @@ -8,7 +8,7 @@ namespace Microsoft.AspNetCore.Components /// /// Indicates that the associated component should match the specified route template pattern. /// - [AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = true)] + [AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = false)] public class RouteAttribute : Attribute { /// diff --git a/src/Components/Components/src/Routing/RouteTable.cs b/src/Components/Components/src/Routing/RouteTable.cs index 0a30f7cbaeba..4449c00f3cc4 100644 --- a/src/Components/Components/src/Routing/RouteTable.cs +++ b/src/Components/Components/src/Routing/RouteTable.cs @@ -1,4 +1,4 @@ -// Copyright (c) .NET Foundation. All rights reserved. +// 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 System; @@ -23,7 +23,12 @@ public static RouteTable Create(IEnumerable types) var routes = new List(); foreach (var type in types) { - var routeAttributes = type.GetCustomAttributes(); // Inherit: true? + // We're deliberately using inherit = false here. + // + // RouteAttribute is defined as non-inherited, because inheriting a route attribute always causes an + // ambiguity. You end up with two components (base class and derived class) with the same route. + var routeAttributes = type.GetCustomAttributes(inherit: false); + foreach (var routeAttribute in routeAttributes) { var template = TemplateParser.ParseTemplate(routeAttribute.Template); From 6317e5b15b428124af782f1b91cb5a0f7d50c93b Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Tue, 14 May 2019 13:08:34 -0700 Subject: [PATCH 2/2] add test --- .../test/Routing/RouteTableTests.cs | 33 +++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/src/Components/Components/test/Routing/RouteTableTests.cs b/src/Components/Components/test/Routing/RouteTableTests.cs index a4144dcfaf5e..830024940ec9 100644 --- a/src/Components/Components/test/Routing/RouteTableTests.cs +++ b/src/Components/Components/test/Routing/RouteTableTests.cs @@ -11,6 +11,39 @@ namespace Microsoft.AspNetCore.Components.Test.Routing { public class RouteTableTests { + [Fact] + public void CanDiscoverRoute() + { + // Arrange & Act + var routes = RouteTable.Create(new[] { typeof(MyComponent), }); + + // Assert + Assert.Equal("Test1", Assert.Single(routes.Routes).Template.TemplateText); + } + + [Route("Test1")] + private class MyComponent : ComponentBase + { + } + + [Fact] + public void CanDiscoverRoutes_WithInheritance() + { + // Arrange & Act + var routes = RouteTable.Create(new[] { typeof(MyComponent), typeof(MyInheritedComponent), }); + + // Assert + Assert.Collection( + routes.Routes.OrderBy(r => r.Template.TemplateText), + r => Assert.Equal("Test1", r.Template.TemplateText), + r => Assert.Equal("Test2", r.Template.TemplateText)); + } + + [Route("Test2")] + private class MyInheritedComponent : MyComponent + { + } + [Fact] public void CanMatchRootTemplate() {