From d794c52c75d11b3d8069e847e3520e40054d7ec2 Mon Sep 17 00:00:00 2001 From: Ryan Nowak <nowakra@gmail.com> Date: Tue, 14 May 2019 15:37:17 -0700 Subject: [PATCH] Make RouteAttribute non-inherited (#10236) * 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. * add test --- ...ft.AspNetCore.Components.netstandard2.0.cs | 2 +- .../Components/src/RouteAttribute.cs | 4 +-- .../Components/src/Routing/RouteTable.cs | 9 +++-- .../test/Routing/RouteTableTests.cs | 33 +++++++++++++++++++ 4 files changed, 43 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 b615df9df1a..1500126c495 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 @@ namespace Microsoft.AspNetCore.Components public System.Threading.Tasks.Task InvokeAsync(System.Func<System.Threading.Tasks.Task> 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 f46d04910fa..a503dab1955 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 /// <summary> /// Indicates that the associated component should match the specified route template pattern. /// </summary> - [AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = true)] + [AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = false)] public class RouteAttribute : Attribute { /// <summary> diff --git a/src/Components/Components/src/Routing/RouteTable.cs b/src/Components/Components/src/Routing/RouteTable.cs index 0a30f7cbaeb..4449c00f3cc 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 @@ namespace Microsoft.AspNetCore.Components.Routing var routes = new List<RouteEntry>(); foreach (var type in types) { - var routeAttributes = type.GetCustomAttributes<RouteAttribute>(); // 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<RouteAttribute>(inherit: false); + foreach (var routeAttribute in routeAttributes) { var template = TemplateParser.ParseTemplate(routeAttribute.Template); diff --git a/src/Components/Components/test/Routing/RouteTableTests.cs b/src/Components/Components/test/Routing/RouteTableTests.cs index a4144dcfaf5..830024940ec 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() { -- GitLab