From 258d34e3828a1870a16d13cd3c62d1b7a65acc4a Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Fri, 5 Apr 2019 08:31:10 -0700 Subject: [PATCH] Use coventional routes for link generation (#9037) Use coventional routes for link generation This change enables using conventional routes for link generation when using MVC conventional routes. This change makes MVC link generation behaviour highly compatible with 2.1. The way that this works is that we create endpoints for **MATCHING** using the denormalized conventional route, but we tell those endpoints to suppress link generation. For link generation we generate a non-matching endpoints per-route with the same order value. I added the concept of *required value any* to link generation. This is needed because for an endpoint to participate in link generation using RouteValuesAddress it needs to have some required values. These details are a little fiddly, but I think it's worth doing this feature completely. --- ...rosoft.AspNetCore.Routing.netcoreapp3.0.cs | 1 + .../Internal/LinkGenerationDecisionTree.cs | 64 +- .../DefaultRoutePatternTransformer.cs | 35 +- src/Http/Routing/src/Patterns/RoutePattern.cs | 21 + .../Routing/src/Template/TemplateBinder.cs | 8 +- .../LinkGenerationDecisionTreeTest.cs | 8 +- .../UnitTests/LinkGeneratorIntegrationTest.cs | 714 ++++++++++++++++++ .../test/UnitTests/LinkGeneratorTestBase.cs | 3 +- .../DefaultRoutePatternTransformerTest.cs | 47 +- ...ontrollerEndpointRouteBuilderExtensions.cs | 8 +- .../MvcCoreServiceCollectionExtensions.cs | 1 - .../src/Routing/ActionEndpointDataSource.cs | 62 -- .../src/Routing/ActionEndpointFactory.cs | 81 +- .../ControllerActionEndpointDataSource.cs | 53 +- .../src/Routing/ConventionalRouteEntry.cs | 17 +- .../Routing/ActionEndpointDataSourceTest.cs | 173 ----- .../test/Routing/ActionEndpointFactoryTest.cs | 11 +- .../ControllerActionEndpointDataSourceTest.cs | 45 +- ...ollerActionEndpointDatasourceBenchmark.cs} | 20 +- .../Mvc.FunctionalTests/LinkGeneratorTest.cs | 22 +- .../test/Mvc.FunctionalTests/RoutingTests.cs | 42 -- .../Mvc.FunctionalTests/RoutingTestsBase.cs | 41 + ...ite.HtmlGeneration_Home.Index.Encoded.html | 10 +- ...tionWebSite.HtmlGeneration_Home.Index.html | 10 +- .../Controllers/LG1Controller.cs | 9 + .../RoutingWebSite/StartupForLinkGenerator.cs | 2 + 26 files changed, 1121 insertions(+), 387 deletions(-) create mode 100644 src/Http/Routing/test/UnitTests/LinkGeneratorIntegrationTest.cs delete mode 100644 src/Mvc/Mvc.Core/src/Routing/ActionEndpointDataSource.cs delete mode 100644 src/Mvc/Mvc.Core/test/Routing/ActionEndpointDataSourceTest.cs rename src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/{ActionEndpointDatasourceBenchmark.cs => ControllerActionEndpointDatasourceBenchmark.cs} (88%) diff --git a/src/Http/Routing/ref/Microsoft.AspNetCore.Routing.netcoreapp3.0.cs b/src/Http/Routing/ref/Microsoft.AspNetCore.Routing.netcoreapp3.0.cs index 81820c85d6..ba1fc3fc6f 100644 --- a/src/Http/Routing/ref/Microsoft.AspNetCore.Routing.netcoreapp3.0.cs +++ b/src/Http/Routing/ref/Microsoft.AspNetCore.Routing.netcoreapp3.0.cs @@ -671,6 +671,7 @@ namespace Microsoft.AspNetCore.Routing.Patterns public sealed partial class RoutePattern { internal RoutePattern() { } + public static readonly object RequiredValueAny; public System.Collections.Generic.IReadOnlyDictionary Defaults { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public decimal InboundPrecedence { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } public decimal OutboundPrecedence { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } } diff --git a/src/Http/Routing/src/Internal/LinkGenerationDecisionTree.cs b/src/Http/Routing/src/Internal/LinkGenerationDecisionTree.cs index a1aee949c9..effc1f0b24 100644 --- a/src/Http/Routing/src/Internal/LinkGenerationDecisionTree.cs +++ b/src/Http/Routing/src/Internal/LinkGenerationDecisionTree.cs @@ -7,6 +7,7 @@ using System.Diagnostics; using System.Linq; using System.Text; using Microsoft.AspNetCore.Routing.DecisionTree; +using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.Routing.Tree; namespace Microsoft.AspNetCore.Routing.Internal @@ -21,38 +22,57 @@ namespace Microsoft.AspNetCore.Routing.Internal private static readonly RouteValueDictionary EmptyAmbientValues = new RouteValueDictionary(); private readonly DecisionTreeNode _root; - private readonly Dictionary> _knownValues; + private readonly List _conventionalEntries; public LinkGenerationDecisionTree(IReadOnlyList entries) { - _root = DecisionTreeBuilder.GenerateTree( - entries, - new OutboundMatchClassifier()); + // We split up the entries into: + // 1. attribute routes - these go into the tree + // 2. conventional routes - these are a list + var attributedEntries = new List(); + _conventionalEntries = new List(); - _knownValues = new Dictionary>(StringComparer.OrdinalIgnoreCase); + // Anything with a RoutePattern.RequiredValueAny as a RequiredValue is a conventional route. + // This is because RequiredValueAny acts as a wildcard, whereas an attribute route entry + // is denormalized to contain an exact set of required values. + // + // We will only see conventional routes show up here for endpoint routing. for (var i = 0; i < entries.Count; i++) { + var isAttributeRoute = true; var entry = entries[i]; foreach (var kvp in entry.Entry.RequiredLinkValues) { - if (!_knownValues.TryGetValue(kvp.Key, out var values)) + if (RoutePattern.IsRequiredValueAny(kvp.Value)) { - values = new HashSet(RouteValueEqualityComparer.Default); - _knownValues.Add(kvp.Key, values); + isAttributeRoute = false; + break; } + } - values.Add(kvp.Value ?? string.Empty); + if (isAttributeRoute) + { + attributedEntries.Add(entry); + } + else + { + _conventionalEntries.Add(entry); } } + + _root = DecisionTreeBuilder.GenerateTree( + attributedEntries, + new OutboundMatchClassifier()); } public IList GetMatches(RouteValueDictionary values, RouteValueDictionary ambientValues) { // Perf: Avoid allocation for List if there aren't any Matches or Criteria - if (_root.Matches.Count > 0 || _root.Criteria.Count > 0) + if (_root.Matches.Count > 0 || _root.Criteria.Count > 0 || _conventionalEntries.Count > 0) { var results = new List(); Walk(results, values, ambientValues ?? EmptyAmbientValues, _root, isFallbackPath: false); + ProcessConventionalEntries(results, values, ambientValues ?? EmptyAmbientValues); results.Sort(OutboundMatchResultComparer.Instance); return results; } @@ -110,19 +130,6 @@ namespace Microsoft.AspNetCore.Routing.Internal { Walk(results, values, ambientValues, branch, isFallbackPath); } - else - { - // If an explicitly specified value doesn't match any branch, then speculatively walk the - // "null" path if the value doesn't match any known value. - // - // This can happen when linking from a page <-> action. We want to be - // able to use "page" and "action" as normal route parameters. - var knownValues = _knownValues[key]; - if (!knownValues.Contains(value ?? string.Empty) && criterion.Branches.TryGetValue(string.Empty, out branch)) - { - Walk(results, values, ambientValues, branch, isFallbackPath: true); - } - } } else { @@ -147,6 +154,17 @@ namespace Microsoft.AspNetCore.Routing.Internal } } + private void ProcessConventionalEntries( + List results, + RouteValueDictionary values, + RouteValueDictionary ambientvalues) + { + for (var i = 0; i < _conventionalEntries.Count; i++) + { + results.Add(new OutboundMatchResult(_conventionalEntries[i], isFallbackMatch: false)); + } + } + private class OutboundMatchClassifier : IClassifier { public IEqualityComparer ValueComparer => RouteValueEqualityComparer.Default; diff --git a/src/Http/Routing/src/Patterns/DefaultRoutePatternTransformer.cs b/src/Http/Routing/src/Patterns/DefaultRoutePatternTransformer.cs index 787e626211..39adf3a4d0 100644 --- a/src/Http/Routing/src/Patterns/DefaultRoutePatternTransformer.cs +++ b/src/Http/Routing/src/Patterns/DefaultRoutePatternTransformer.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; @@ -43,8 +43,9 @@ namespace Microsoft.AspNetCore.Routing.Patterns { // There are three possible cases here: // 1. Required value is null-ish - // 2. Required value corresponds to a parameter - // 3. Required value corresponds to a matching default value + // 2. Required value is *any* + // 3. Required value corresponds to a parameter + // 4. Required value corresponds to a matching default value // // If none of these are true then we can reject this substitution. RoutePatternParameterPart parameter; @@ -76,9 +77,28 @@ namespace Microsoft.AspNetCore.Routing.Patterns // Ex: {controller=Home}/{action=Index}/{id?} - with required values: { area = "", ... } continue; } + else if (RoutePattern.IsRequiredValueAny(kvp.Value)) + { + // 2. Required value is *any* - this is allowed for a parameter with a default, but not + // a non-parameter default. + if (original.GetParameter(kvp.Key) == null && + original.Defaults.TryGetValue(kvp.Key, out var defaultValue) && + !RouteValueEqualityComparer.Default.Equals(string.Empty, defaultValue)) + { + // Fail: this route as a non-parameter default that is stricter than *any*. + // + // Ex: Admin/{controller=Home}/{action=Index}/{id?} defaults: { area = "Admin" } - with required values: { area = *any* } + return null; + } + + // Success: (for this parameter at least) + // + // Ex: {controller=Home}/{action=Index}/{id?} - with required values: { controller = *any*, ... } + continue; + } else if ((parameter = original.GetParameter(kvp.Key)) != null) { - // 2. Required value corresponds to a parameter - check to make sure that this value matches + // 3. Required value corresponds to a parameter - check to make sure that this value matches // any IRouteConstraint implementations. if (!MatchesConstraints(original, parameter, kvp.Key, requiredValues)) { @@ -96,7 +116,7 @@ namespace Microsoft.AspNetCore.Routing.Patterns else if (original.Defaults.TryGetValue(kvp.Key, out var defaultValue) && RouteValueEqualityComparer.Default.Equals(kvp.Value, defaultValue)) { - // 3. Required value corresponds to a matching default value - check to make sure that this value matches + // 4. Required value corresponds to a matching default value - check to make sure that this value matches // any IRouteConstraint implementations. It's unlikely that this would happen in practice but it doesn't // hurt for us to check. if (!MatchesConstraints(original, parameter: null, kvp.Key, requiredValues)) @@ -142,7 +162,10 @@ namespace Microsoft.AspNetCore.Routing.Patterns // We only need to handle the case where the required value maps to a parameter. That's the only // case where we allow a default and a required value to disagree, and we already validated the // other cases. - if (parameter != null && + // + // If the required value is *any* then don't remove the default. + if (parameter != null && + !RoutePattern.IsRequiredValueAny(kvp.Value) && original.Defaults.TryGetValue(kvp.Key, out var defaultValue) && !RouteValueEqualityComparer.Default.Equals(kvp.Value, defaultValue)) { diff --git a/src/Http/Routing/src/Patterns/RoutePattern.cs b/src/Http/Routing/src/Patterns/RoutePattern.cs index 6852140bad..b500f1d2f7 100644 --- a/src/Http/Routing/src/Patterns/RoutePattern.cs +++ b/src/Http/Routing/src/Patterns/RoutePattern.cs @@ -17,6 +17,21 @@ namespace Microsoft.AspNetCore.Routing.Patterns [DebuggerDisplay("{DebuggerToString()}")] public sealed class RoutePattern { + /// + /// A marker object that can be used in to designate that + /// any non-null or non-empty value is required. + /// + /// + /// is only use in routing is in . + /// is not valid as a route value, and will convert to the null/empty string. + /// + public static readonly object RequiredValueAny = new RequiredValueAnySentinal(); + + internal static bool IsRequiredValueAny(object value) + { + return object.ReferenceEquals(RequiredValueAny, value); + } + private const string SeparatorString = "/"; internal RoutePattern( @@ -140,5 +155,11 @@ namespace Microsoft.AspNetCore.Routing.Patterns { return RawText ?? string.Join(SeparatorString, PathSegments.Select(s => s.DebuggerToString())); } + + [DebuggerDisplay("{DebuggerToString(),nq}")] + private class RequiredValueAnySentinal + { + private string DebuggerToString() => "*any*"; + } } } diff --git a/src/Http/Routing/src/Template/TemplateBinder.cs b/src/Http/Routing/src/Template/TemplateBinder.cs index d7a93ba0ba..c78573ab98 100644 --- a/src/Http/Routing/src/Template/TemplateBinder.cs +++ b/src/Http/Routing/src/Template/TemplateBinder.cs @@ -178,7 +178,8 @@ namespace Microsoft.AspNetCore.Routing.Template throw new InvalidOperationException($"Unable to find required value '{key}' on route pattern."); } - if (!RoutePartsEqual(ambientValue, _pattern.RequiredValues[key])) + if (!RoutePartsEqual(ambientValue, _pattern.RequiredValues[key]) && + !RoutePattern.IsRequiredValueAny(_pattern.RequiredValues[key])) { copyAmbientValues = false; break; @@ -261,10 +262,11 @@ namespace Microsoft.AspNetCore.Routing.Template // // OR in plain English... when linking from a page in an area to an action in the same area, it should // be possible to use the area as an ambient value. - if (!copyAmbientValues && _pattern.RequiredValues.TryGetValue(key, out var requiredValue)) + if (!copyAmbientValues && !hasExplicitValue && _pattern.RequiredValues.TryGetValue(key, out var requiredValue)) { hasAmbientValue = ambientValues != null && ambientValues.TryGetValue(key, out ambientValue); - if (hasAmbientValue && RoutePartsEqual(requiredValue, ambientValue)) + if (hasAmbientValue && + (RoutePartsEqual(requiredValue, ambientValue) || RoutePattern.IsRequiredValueAny(requiredValue))) { // Treat this an an explicit value to *force it*. slots[i] = new KeyValuePair(key, ambientValue); diff --git a/src/Http/Routing/test/UnitTests/Internal/LinkGenerationDecisionTreeTest.cs b/src/Http/Routing/test/UnitTests/Internal/LinkGenerationDecisionTreeTest.cs index effcda3bd8..b141386a68 100644 --- a/src/Http/Routing/test/UnitTests/Internal/LinkGenerationDecisionTreeTest.cs +++ b/src/Http/Routing/test/UnitTests/Internal/LinkGenerationDecisionTreeTest.cs @@ -583,9 +583,7 @@ namespace Microsoft.AspNetCore.Routing.Internal.Routing var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList(); // Assert - Assert.Collection( - matches, - m => { Assert.Same(entry1, m); }); + Assert.Empty(matches); } [Fact] @@ -689,9 +687,7 @@ namespace Microsoft.AspNetCore.Routing.Internal.Routing var matches = tree.GetMatches(context.Values, context.AmbientValues).Select(m => m.Match).ToList(); // Assert - Assert.Collection( - matches, - m => { Assert.Same(entry2, m); }); + Assert.Empty(matches); } [Fact] diff --git a/src/Http/Routing/test/UnitTests/LinkGeneratorIntegrationTest.cs b/src/Http/Routing/test/UnitTests/LinkGeneratorIntegrationTest.cs new file mode 100644 index 0000000000..c003079640 --- /dev/null +++ b/src/Http/Routing/test/UnitTests/LinkGeneratorIntegrationTest.cs @@ -0,0 +1,714 @@ +// 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 Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Http.Features; +using Microsoft.AspNetCore.Routing.Patterns; +using System.Collections.Generic; +using Xunit; + +namespace Microsoft.AspNetCore.Routing +{ + // This is a set of integration tests that are similar to a typical MVC configuration. + // + // We're doing this here because it's relatively expensive to test these scenarios + // inside MVC - it requires creating actual controllers and pages. + public class LinkGeneratorIntegrationTest : LinkGeneratorTestBase + { + public LinkGeneratorIntegrationTest() + { + var endpoints = new List() + { + // Attribute routed endpoint 1 + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "api/Pets/{id}", + defaults: new { controller = "Pets", action = "GetById", }, + parameterPolicies: null, + requiredValues: new { controller = "Pets", action = "GetById", area = (string)null, page = (string)null, }), + order: 0), + + // Attribute routed endpoint 2 + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "api/Pets", + defaults: new { controller = "Pets", action = "GetAll", }, + parameterPolicies: null, + requiredValues: new { controller = "Pets", action = "GetAll", area = (string)null, page = (string)null, }), + order: 0), + + // Attribute routed endpoint 2 + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "api/Pets/{id}", + defaults: new { controller = "Pets", action = "Update", }, + parameterPolicies: null, + requiredValues: new { controller = "Pets", action = "Update", area = (string)null, page = (string)null, }), + order: 0), + + // Attribute routed endpoint 4 + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "api/Inventory/{searchTerm}/{page}", + defaults: new { controller = "Inventory", action = "Search", }, + parameterPolicies: null, + requiredValues: new { controller = "Inventory", action = "Search", area = (string)null, page = (string)null, }), + order: 0), + + // Conventional routed endpoint 1 + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "{controller=Home}/{action=Index}/{id?}", + defaults: null, + parameterPolicies: null, + requiredValues: new { controller = "Home", action = "Index", area = (string)null, page = (string)null, }), + order: 2000, + metadata: new object[] { new SuppressLinkGenerationMetadata(), }), + + // Conventional routed endpoint 2 + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "{controller=Home}/{action=Index}/{id?}", + defaults: null, + parameterPolicies: null, + requiredValues: new { controller = "Home", action = "About", area = (string)null, page = (string)null, }), + order: 2000, + metadata: new object[] { new SuppressLinkGenerationMetadata(), }), + + // Conventional routed endpoint 3 + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "{controller=Home}/{action=Index}/{id?}", + defaults: null, + parameterPolicies: null, + requiredValues: new { controller = "Store", action = "Browse", area = (string)null, page = (string)null, }), + order: 2000, + metadata: new object[] { new SuppressLinkGenerationMetadata(), }), + + // Conventional routed link generation route 1 + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "{controller=Home}/{action=Index}/{id?}", + defaults: null, + parameterPolicies: null, + requiredValues: new { controller = RoutePattern.RequiredValueAny, action = RoutePattern.RequiredValueAny, area = (string)null, page = (string)null, }), + order: 2000, + metadata: new object[] { new SuppressMatchingMetadata(), }), + + // Conventional routed endpoint 4 (with area) + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "Admin/{controller=Home}/{action=Index}/{id?}", + defaults: new { area = "Admin", }, + parameterPolicies: new { controller = "Admin", }, + requiredValues: new { area = "Admin", controller = "Users", action = "Add", page = (string)null, }), + order: 1000, + metadata: new object[] { new SuppressLinkGenerationMetadata(), }), + + // Conventional routed endpoint 5 (with area) + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "Admin/{controller=Home}/{action=Index}/{id?}", + defaults: new { area = "Admin", }, + parameterPolicies: new { controller = "Admin", }, + requiredValues: new { area = "Admin", controller = "Users", action = "Remove", page = (string)null, }), + order: 1000, + metadata: new object[] { new SuppressLinkGenerationMetadata(), }), + + // Conventional routed link generation route 2 + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "Admin/{controller=Home}/{action=Index}/{id?}", + defaults: new { area = "Admin", }, + parameterPolicies: new { area = "Admin", }, + requiredValues: new { controller = RoutePattern.RequiredValueAny, action = RoutePattern.RequiredValueAny, area = "Admin", page = (string)null, }), + order: 1000, + metadata: new object[] { new SuppressMatchingMetadata(), }), + + // Conventional routed link generation route 3 - this doesn't match any actions. + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "api/{controller}/{id?}", + defaults: new { }, + parameterPolicies: new { }, + requiredValues: new { controller = RoutePattern.RequiredValueAny, action = (string)null, area = (string)null, page = (string)null, }), + order: 3000, + metadata: new object[] { new SuppressMatchingMetadata(), new RouteNameMetadata("custom"), }), + + // Conventional routed link generation route 3 - this doesn't match any actions. + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "api/Foo/{custom2}", + defaults: new { }, + parameterPolicies: new { }, + requiredValues: new { controller = (string)null, action = (string)null, area = (string)null, page = (string)null, }), + order: 3000, + metadata: new object[] { new SuppressMatchingMetadata(), new RouteNameMetadata("custom2"), }), + + // Razor Page 1 primary endpoint + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "Pages", + defaults: new { page = "/Pages/Index", }, + parameterPolicies: null, + requiredValues: new { controller = (string)null, action = (string)null, area = (string)null, page = "/Pages/Index", }), + order: 0), + + // Razor Page 1 secondary endpoint + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "Pages/Index", + defaults: new { page = "/Pages/Index", }, + parameterPolicies: null, + requiredValues: new { controller = (string)null, action = (string)null, area = (string)null, page = "/Pages/Index", }), + order: 0, + metadata: new object[] { new SuppressLinkGenerationMetadata(), }), + + // Razor Page 2 primary endpoint + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "Pages/Help/{id?}", + defaults: new { page = "/Pages/Help", }, + parameterPolicies: null, + requiredValues: new { controller = (string)null, action = (string)null, area = (string)null, page = "/Pages/Help", }), + order: 0), + + // Razor Page 3 primary endpoint + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "Pages/About/{id?}", + defaults: new { page = "/Pages/About", }, + parameterPolicies: null, + requiredValues: new { controller = (string)null, action = (string)null, area = (string)null, page = "/Pages/About", }), + order: 0), + + // Razor Page 4 with area primary endpoint + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "Admin/Pages", + defaults: new { page = "/Pages/Index", area = "Admin", }, + parameterPolicies: null, + requiredValues: new { controller = (string)null, action = (string)null, area = "Admin", page = "/Pages/Index", }), + order: 0), + + // Razor Page 4 with area secondary endpoint + EndpointFactory.CreateRouteEndpoint( + RoutePatternFactory.Parse( + "Admin/Pages/Index", + defaults: new { page = "/Pages/Index", area = "Admin", }, + parameterPolicies: null, + requiredValues: new { controller = (string)null, action = (string)null, area = "Admin", page = "/Pages/Index", }), + order: 0, + metadata: new object[] { new SuppressLinkGenerationMetadata(), }), + }; + + Endpoints = endpoints; + LinkGenerator = CreateLinkGenerator(endpoints.ToArray()); + } + + private IReadOnlyList Endpoints { get; } + + private LinkGenerator LinkGenerator { get; } + + #region Without ambient values (simple cases) + + [Fact] + public void GetPathByAddress_LinkToAttributedAction_GeneratesPath() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Pets", action = "GetById", id = "17", }; + var ambientValues = new { }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/api/Pets/17", path); + } + + [Fact] + public void GetPathByAddress_LinkToConventionalAction_GeneratesPath() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Home", action = "Index", }; + var ambientValues = new { }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/", path); + } + + [Fact] + public void GetPathByAddress_LinkToConventionalActionInArea_GeneratesPath() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { area = "Admin", controller = "Users", action = "Add", }; + var ambientValues = new { }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Admin/Users/Add", path); + } + + [Fact] + public void GetPathByAddress_LinkToConventionalRoute_GeneratesPath() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Store", id = "17", }; + var ambientValues = new { }; + var address = CreateAddress(routeName: "custom", values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/api/Store/17", path); + } + + [Fact] + public void GetPathByAddress_LinkToPage_GeneratesPath() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { page = "/Pages/Index", }; + var ambientValues = new { }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Pages", path); + } + + [Fact] + public void GetPathByAddress_LinkToPageInArea_GeneratesPath() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { area = "Admin", page = "/Pages/Index", }; + var ambientValues = new { }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Admin/Pages", path); + } + + [Fact] + public void GetPathByAddress_LinkToNonExistentAction_GeneratesPath() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Home", action = "Fake", id = "17", }; + var ambientValues = new { }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Home/Fake/17", path); + } + + #endregion + + #region With ambient values + + [Fact] + public void GetPathByAddress_LinkToAttributedAction_FromSameAction_KeepsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Pets", action = "GetById", }; + var ambientValues = new { controller = "Pets", action = "GetById", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/api/Pets/17", path); + } + + [Fact] + public void GetPathByAddress_LinkToAttributedAction_FromAnotherAction_DiscardsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Pets", action = "GetById", }; + var ambientValues = new { controller = "Pets", action = "Update", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Pets/GetById", path); + } + + [Fact] + public void GetPathByAddress_LinkToAttributedAction_FromPage_DiscardsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Pets", action = "GetById", }; + var ambientValues = new { page = "/Pages/Help", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Pets/GetById", path); + } + + [Fact] + public void GetPathByAddress_LinkToConventionalAction_FromSameAction_KeepsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Home", action = "Index", }; + var ambientValues = new { controller = "Home", action = "Index", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Home/Index/17", path); + } + + [Fact] + public void GetPathByAddress_LinkToConventionalAction_FromAnotherAction_DiscardsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Home", action = "Index", }; + var ambientValues = new { controller = "Pets", action = "Update", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/", path); + } + + [Fact] + public void GetPathByAddress_LinkToConventionalAction_FromPage_DiscardsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Home", action = "Index", }; + var ambientValues = new { page = "/Pages/Help", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/", path); + } + + [Fact] + public void GetPathByAddress_LinkToNonExistentConventionalAction_FromAnotherAction_DiscardsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Home", action = "Index11", }; + var ambientValues = new { controller = "Pets", action = "Update", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Home/Index11", path); + } + + [Fact] + public void GetPathByAddress_LinkToNonExistentAreaAction_FromAnotherAction_DiscardsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { area = "Admin", controller = "Home", action = "Index11", }; + var ambientValues = new { controller = "Pets", action = "Update", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Admin/Home/Index11", path); + } + + [Fact] + public void GetPathByAddress_LinkToConventionalRoute_FromAction_DiscardsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Store", }; + var ambientValues = new { controller = "Home", action = "Index", id = "17", }; + var address = CreateAddress(routeName: "custom", values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/api/Store", path); + } + + [Fact] + public void GetPathByAddress_LinkToConventionalRoute_WithAmbientValues_GeneratesPath() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { controller = "Store", id = "17", }; + var ambientValues = new { controller = "Store", }; + var address = CreateAddress(routeName: "custom", values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/api/Store/17", path); + } + + [Fact] + public void GetPathByAddress_LinkToConventionalRouteWithoutSharedAmbientValues_WithAmbientValues_GeneratesPath() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { custom2 = "17", }; + var ambientValues = new { controller = "Store", }; + var address = CreateAddress(routeName: "custom2", values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/api/Foo/17", path); + } + + [Fact] + public void GetPathByAddress_LinkToPage_FromSamePage_KeepsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { page = "/Pages/Help", }; + var ambientValues = new { page = "/Pages/Help", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Pages/Help/17", path); + } + + [Fact] + public void GetPathByAddress_LinkToPage_FromAction_DiscardsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { page = "/Pages/Help", }; + var ambientValues = new { controller = "Pets", action = "Update", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Pages/Help", path); + } + + [Fact] + public void GetPathByAddress_LinkToPage_FromAnotherPage_DiscardsAmbientValues() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { page = "/Pages/Help", }; + var ambientValues = new { page = "/Pages/About", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Pages/Help", path); + } + + [Fact] + public void GetPathByAddress_LinkToNonExistentPage_FromAction_MatchesActionConventionalRoute() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { page = "/Pages/Help2", }; + var ambientValues = new { controller = "Pets", action = "Update", id = "17", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Pets/Update?page=%2FPages%2FHelp2", path); + } + + [Fact] + public void GetPathByAddress_LinkToPageInSameArea_FromAction_UsingAreaAmbientValue() + { + // Arrange + var httpContext = CreateHttpContext(); + + var values = new { page = "/Pages/Index", }; + var ambientValues = new { area = "Admin", controller = "Users", action = "Add", }; + var address = CreateAddress(values: values, ambientValues: ambientValues); + + // Act + var path = LinkGenerator.GetPathByAddress( + httpContext, + address, + address.ExplicitValues, + address.AmbientValues); + + // Assert + Assert.Equal("/Admin/Pages", path); + } + + #endregion + + private static RouteValuesAddress CreateAddress(string routeName = null, object values = null, object ambientValues = null) + { + return new RouteValuesAddress() + { + RouteName = routeName, + ExplicitValues = new RouteValueDictionary(values), + AmbientValues = new RouteValueDictionary(ambientValues), + }; + } + } +} diff --git a/src/Http/Routing/test/UnitTests/LinkGeneratorTestBase.cs b/src/Http/Routing/test/UnitTests/LinkGeneratorTestBase.cs index 117a6695ac..202238e5b8 100644 --- a/src/Http/Routing/test/UnitTests/LinkGeneratorTestBase.cs +++ b/src/Http/Routing/test/UnitTests/LinkGeneratorTestBase.cs @@ -1,6 +1,7 @@ // 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; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Routing.Internal; @@ -8,8 +9,6 @@ using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.ObjectPool; using Microsoft.Extensions.Options; -using System; -using System.Collections.Generic; namespace Microsoft.AspNetCore.Routing { diff --git a/src/Http/Routing/test/UnitTests/Patterns/DefaultRoutePatternTransformerTest.cs b/src/Http/Routing/test/UnitTests/Patterns/DefaultRoutePatternTransformerTest.cs index 9a0145038e..7ca0bce627 100644 --- a/src/Http/Routing/test/UnitTests/Patterns/DefaultRoutePatternTransformerTest.cs +++ b/src/Http/Routing/test/UnitTests/Patterns/DefaultRoutePatternTransformerTest.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 Microsoft.AspNetCore.Routing.Constraints; @@ -62,6 +62,32 @@ namespace Microsoft.AspNetCore.Routing.Patterns Assert.Null(actual); } + [Fact] + public void SubstituteRequiredValues_AllowRequiredValueAnyForParameter() + { + // Arrange + var template = "{controller=Home}/{action=Index}/{id?}"; + var defaults = new { }; + var policies = new { }; + + var original = RoutePatternFactory.Parse(template, defaults, policies); + + var requiredValues = new { controller = RoutePattern.RequiredValueAny, }; + + // Act + var actual = Transformer.SubstituteRequiredValues(original, requiredValues); + + // Assert + Assert.Collection( + actual.Defaults.OrderBy(kvp => kvp.Key), + kvp => Assert.Equal(new KeyValuePair("action", "Index"), kvp), + kvp => Assert.Equal(new KeyValuePair("controller", "Home"), kvp)); // default is preserved + + Assert.Collection( + actual.RequiredValues.OrderBy(kvp => kvp.Key), + kvp => Assert.Equal(new KeyValuePair("controller", RoutePattern.RequiredValueAny), kvp)); + } + [Fact] public void SubstituteRequiredValues_RejectsNullForOutOfLineDefault() { @@ -81,6 +107,25 @@ namespace Microsoft.AspNetCore.Routing.Patterns Assert.Null(actual); } + [Fact] + public void SubstituteRequiredValues_RejectsRequiredValueAnyForOutOfLineDefault() + { + // Arrange + var template = "{controller=Home}/{action=Index}/{id?}"; + var defaults = new { area = RoutePattern.RequiredValueAny }; + var policies = new { }; + + var original = RoutePatternFactory.Parse(template, defaults, policies); + + var requiredValues = new { area = string.Empty, }; + + // Act + var actual = Transformer.SubstituteRequiredValues(original, requiredValues); + + // Assert + Assert.Null(actual); + } + [Fact] public void SubstituteRequiredValues_CanAcceptValueForParameter() { diff --git a/src/Mvc/Mvc.Core/src/Builder/ControllerEndpointRouteBuilderExtensions.cs b/src/Mvc/Mvc.Core/src/Builder/ControllerEndpointRouteBuilderExtensions.cs index 726988fd6f..01569e59dd 100644 --- a/src/Mvc/Mvc.Core/src/Builder/ControllerEndpointRouteBuilderExtensions.cs +++ b/src/Mvc/Mvc.Core/src/Builder/ControllerEndpointRouteBuilderExtensions.cs @@ -50,12 +50,12 @@ namespace Microsoft.AspNetCore.Builder EnsureControllerServices(endpoints); var dataSource = GetOrCreateDataSource(endpoints); - dataSource.AddRoute(new ConventionalRouteEntry( + dataSource.AddRoute( "default", "{controller=Home}/{action=Index}/{id?}", defaults: null, constraints: null, - dataTokens: null)); + dataTokens: null); return dataSource; } @@ -96,12 +96,12 @@ namespace Microsoft.AspNetCore.Builder EnsureControllerServices(endpoints); var dataSource = GetOrCreateDataSource(endpoints); - dataSource.AddRoute(new ConventionalRouteEntry( + dataSource.AddRoute( name, pattern, new RouteValueDictionary(defaults), new RouteValueDictionary(constraints), - new RouteValueDictionary(dataTokens))); + new RouteValueDictionary(dataTokens)); } /// diff --git a/src/Mvc/Mvc.Core/src/DependencyInjection/MvcCoreServiceCollectionExtensions.cs b/src/Mvc/Mvc.Core/src/DependencyInjection/MvcCoreServiceCollectionExtensions.cs index 2ab9cb64f0..380dafdc68 100644 --- a/src/Mvc/Mvc.Core/src/DependencyInjection/MvcCoreServiceCollectionExtensions.cs +++ b/src/Mvc/Mvc.Core/src/DependencyInjection/MvcCoreServiceCollectionExtensions.cs @@ -269,7 +269,6 @@ namespace Microsoft.Extensions.DependencyInjection // // Endpoint Routing / Endpoints // - services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); services.TryAddSingleton(); diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointDataSource.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointDataSource.cs deleted file mode 100644 index b5c0eb10e5..0000000000 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointDataSource.cs +++ /dev/null @@ -1,62 +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 System; -using System.Collections.Generic; -using Microsoft.AspNetCore.Builder; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc.Abstractions; -using Microsoft.AspNetCore.Mvc.Infrastructure; - -namespace Microsoft.AspNetCore.Mvc.Routing -{ - internal class ActionEndpointDataSource : ActionEndpointDataSourceBase - { - private readonly ActionEndpointFactory _endpointFactory; - private readonly List _routes; - - public ActionEndpointDataSource(IActionDescriptorCollectionProvider actions, ActionEndpointFactory endpointFactory) - : base(actions) - { - _endpointFactory = endpointFactory; - - _routes = new List(); - - // IMPORTANT: this needs to be the last thing we do in the constructor. - // Change notifications can happen immediately! - Subscribe(); - } - - // For testing - public IReadOnlyList Routes - { - get - { - lock (Lock) - { - return _routes.ToArray(); - } - } - } - - public void AddRoute(in ConventionalRouteEntry route) - { - lock (Lock) - { - _routes.Add(route); - } - } - - protected override List CreateEndpoints(IReadOnlyList actions, IReadOnlyList> conventions) - { - var endpoints = new List(); - for (var i = 0; i < actions.Count; i++) - { - _endpointFactory.AddEndpoints(endpoints, actions[i], _routes, conventions); - } - - return endpoints; - } - } -} - diff --git a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs index 48a38655bf..ed1e2f5f42 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ActionEndpointFactory.cs @@ -4,6 +4,7 @@ using System; using System.Collections.Generic; using System.Linq; +using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; @@ -58,14 +59,6 @@ namespace Microsoft.AspNetCore.Mvc.Routing if (action.AttributeRouteInfo == null) { - // In traditional conventional routing setup, the routes defined by a user have a static order - // defined by how they are added into the list. We would like to maintain the same order when building - // up the endpoints too. - // - // Start with an order of '1' for conventional routes as attribute routes have a default order of '0'. - // This is for scenarios dealing with migrating existing Router based code to Endpoint Routing world. - var conventionalRouteOrder = 1; - // Check each of the conventional patterns to see if the action would be reachable. // If the action and pattern are compatible then create an endpoint with action // route values on the pattern. @@ -80,13 +73,15 @@ namespace Microsoft.AspNetCore.Mvc.Routing continue; } + // We suppress link generation for each conventionally routed endpoint. We generate a single endpoint per-route + // to handle link generation. var builder = CreateEndpoint( action, updatedRoutePattern, route.RouteName, - conventionalRouteOrder++, + route.Order, route.DataTokens, - suppressLinkGeneration: false, + suppressLinkGeneration: true, suppressPathMatching: false, conventions); endpoints.Add(builder); @@ -119,6 +114,72 @@ namespace Microsoft.AspNetCore.Mvc.Routing } } + public void AddConventionalLinkGenerationRoute(List endpoints, HashSet keys, ConventionalRouteEntry route, IReadOnlyList> conventions) + { + if (endpoints == null) + { + throw new ArgumentNullException(nameof(endpoints)); + } + + if (keys == null) + { + throw new ArgumentNullException(nameof(keys)); + } + + if (conventions == null) + { + throw new ArgumentNullException(nameof(conventions)); + } + + var requiredValues = new RouteValueDictionary(); + foreach (var key in keys) + { + if (route.Pattern.GetParameter(key) != null) + { + // Parameter (allow any) + requiredValues[key] = RoutePattern.RequiredValueAny; + } + else if (route.Pattern.Defaults.TryGetValue(key, out var value)) + { + requiredValues[key] = value; + } + else + { + requiredValues[key] = null; + } + } + + // We have to do some massaging of the pattern to try and get the + // required values to be correct. + var pattern = _routePatternTransformer.SubstituteRequiredValues(route.Pattern, requiredValues); + if (pattern == null) + { + // We don't expect this to happen, but we want to know if it does because it will help diagnose the bug. + throw new InvalidOperationException("Failed to create a conventional route for pattern: " + route.Pattern); + } + + var builder = new RouteEndpointBuilder(context => Task.CompletedTask, pattern, route.Order) + { + DisplayName = "Route: " + route.Pattern.RawText, + Metadata = + { + new SuppressMatchingMetadata(), + }, + }; + + if (route.RouteName != null) + { + builder.Metadata.Add(new RouteNameMetadata(route.RouteName)); + } + + for (var i = 0; i < conventions.Count; i++) + { + conventions[i](builder); + } + + endpoints.Add((RouteEndpoint)builder.Build()); + } + private static (RoutePattern resolvedRoutePattern, IDictionary resolvedRequiredValues) ResolveDefaultsAndRequiredValues(ActionDescriptor action, RoutePattern attributeRoutePattern) { RouteValueDictionary updatedDefaults = null; diff --git a/src/Mvc/Mvc.Core/src/Routing/ControllerActionEndpointDataSource.cs b/src/Mvc/Mvc.Core/src/Routing/ControllerActionEndpointDataSource.cs index 9a1ce02cd8..98f2bd474b 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ControllerActionEndpointDataSource.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ControllerActionEndpointDataSource.cs @@ -3,11 +3,14 @@ using System; using System.Collections.Generic; +using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Mvc.Controllers; using Microsoft.AspNetCore.Mvc.Infrastructure; +using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; namespace Microsoft.AspNetCore.Mvc.Routing { @@ -16,37 +19,81 @@ namespace Microsoft.AspNetCore.Mvc.Routing private readonly ActionEndpointFactory _endpointFactory; private readonly List _routes; - public ControllerActionEndpointDataSource(IActionDescriptorCollectionProvider actions, ActionEndpointFactory endpointFactory) + private int _order; + + public ControllerActionEndpointDataSource( + IActionDescriptorCollectionProvider actions, + ActionEndpointFactory endpointFactory) : base(actions) { _endpointFactory = endpointFactory; _routes = new List(); + // In traditional conventional routing setup, the routes defined by a user have a order + // defined by how they are added into the list. We would like to maintain the same order when building + // up the endpoints too. + // + // Start with an order of '1' for conventional routes as attribute routes have a default order of '0'. + // This is for scenarios dealing with migrating existing Router based code to Endpoint Routing world. + _order = 1; + // IMPORTANT: this needs to be the last thing we do in the constructor. // Change notifications can happen immediately! Subscribe(); } - public void AddRoute(in ConventionalRouteEntry route) + + public void AddRoute( + string routeName, + string pattern, + RouteValueDictionary defaults, + IDictionary constraints, + RouteValueDictionary dataTokens) { lock (Lock) { - _routes.Add(route); + _routes.Add(new ConventionalRouteEntry(routeName, pattern, defaults, constraints, dataTokens, _order++)); } } protected override List CreateEndpoints(IReadOnlyList actions, IReadOnlyList> conventions) { var endpoints = new List(); + var keys = new HashSet(StringComparer.OrdinalIgnoreCase); + + // For each controller action - add the relevant endpoints. + // + // 1. If the action is attribute routed, we use that information verbatim + // 2. If the action is conventional routed + // a. Create a *matching only* endpoint for each action X route (if possible) + // b. Ignore link generation for now for (var i = 0; i < actions.Count; i++) { if (actions[i] is ControllerActionDescriptor action) { _endpointFactory.AddEndpoints(endpoints, action, _routes, conventions); + + if (_routes.Count > 0) + { + // If we have conventional routes, keep track of the keys so we can create + // the link generation routes later. + foreach (var kvp in action.RouteValues) + { + keys.Add(kvp.Key); + } + } } } + // Now create a *link generation only* endpoint for each route. This gives us a very + // compatible experience to previous versions. + for (var i = 0; i < _routes.Count; i++) + { + var route = _routes[i]; + _endpointFactory.AddConventionalLinkGenerationRoute(endpoints, keys, route, conventions); + } + return endpoints; } } diff --git a/src/Mvc/Mvc.Core/src/Routing/ConventionalRouteEntry.cs b/src/Mvc/Mvc.Core/src/Routing/ConventionalRouteEntry.cs index 18d587daac..b8cfd45564 100644 --- a/src/Mvc/Mvc.Core/src/Routing/ConventionalRouteEntry.cs +++ b/src/Mvc/Mvc.Core/src/Routing/ConventionalRouteEntry.cs @@ -14,16 +14,19 @@ namespace Microsoft.AspNetCore.Mvc.Routing public readonly RoutePattern Pattern; public readonly string RouteName; public readonly RouteValueDictionary DataTokens; + public readonly int Order; public ConventionalRouteEntry( string routeName, string pattern, RouteValueDictionary defaults, IDictionary constraints, - RouteValueDictionary dataTokens) + RouteValueDictionary dataTokens, + int order) { RouteName = routeName; DataTokens = dataTokens; + Order = order; try { @@ -40,17 +43,5 @@ namespace Microsoft.AspNetCore.Mvc.Routing pattern), exception); } } - - public ConventionalRouteEntry(RoutePattern pattern, string routeName, RouteValueDictionary dataTokens) - { - if (pattern == null) - { - throw new ArgumentNullException(nameof(pattern)); - } - - Pattern = pattern; - RouteName = routeName; - DataTokens = dataTokens; - } } } diff --git a/src/Mvc/Mvc.Core/test/Routing/ActionEndpointDataSourceTest.cs b/src/Mvc/Mvc.Core/test/Routing/ActionEndpointDataSourceTest.cs deleted file mode 100644 index 0cc3ab7959..0000000000 --- a/src/Mvc/Mvc.Core/test/Routing/ActionEndpointDataSourceTest.cs +++ /dev/null @@ -1,173 +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 System; -using System.Collections.Generic; -using System.Linq; -using Microsoft.AspNetCore.Mvc.Abstractions; -using Microsoft.AspNetCore.Mvc.Infrastructure; -using Microsoft.AspNetCore.Routing; -using Moq; -using Xunit; - -namespace Microsoft.AspNetCore.Mvc.Routing -{ - public class ActionEndpointDataSourceTest : ActionEndpointDataSourceBaseTest - { - [Fact] - public void Endpoints_MultipledActions_MultipleRoutes() - { - // Arrange - var actions = new List - { - new ActionDescriptor - { - AttributeRouteInfo = new AttributeRouteInfo() - { - Template = "/test", - }, - RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { "action", "Test" }, - { "controller", "Test" }, - }, - }, - new ActionDescriptor - { - RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { "action", "Index" }, - { "controller", "Home" }, - }, - } - }; - - var mockDescriptorProvider = new Mock(); - mockDescriptorProvider - .Setup(m => m.ActionDescriptors) - .Returns(new ActionDescriptorCollection(actions, 0)); - - var dataSource = (ActionEndpointDataSource)CreateDataSource(mockDescriptorProvider.Object); - dataSource.AddRoute(new ConventionalRouteEntry("1", "/1/{controller}/{action}/{id?}", null, null, null)); - dataSource.AddRoute(new ConventionalRouteEntry("2", "/2/{controller}/{action}/{id?}", null, null, null)); - - // Act - var endpoints = dataSource.Endpoints; - - // Assert - Assert.Collection( - endpoints.Cast().OrderBy(e => e.RoutePattern.RawText), - e => - { - Assert.Equal("/1/{controller}/{action}/{id?}", e.RoutePattern.RawText); - Assert.Same(actions[1], e.Metadata.GetMetadata()); - }, - e => - { - Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText); - Assert.Same(actions[1], e.Metadata.GetMetadata()); - }, - e => - { - Assert.Equal("/test", e.RoutePattern.RawText); - Assert.Same(actions[0], e.Metadata.GetMetadata()); - }); - } - - [Fact] - public void Endpoints_AppliesConventions() - { - // Arrange - var actions = new List - { - new ActionDescriptor - { - AttributeRouteInfo = new AttributeRouteInfo() - { - Template = "/test", - }, - RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { "action", "Test" }, - { "controller", "Test" }, - }, - }, - new ActionDescriptor - { - RouteValues = new Dictionary(StringComparer.OrdinalIgnoreCase) - { - { "action", "Index" }, - { "controller", "Home" }, - }, - } - }; - - var mockDescriptorProvider = new Mock(); - mockDescriptorProvider.Setup(m => m.ActionDescriptors).Returns(new ActionDescriptorCollection(actions, 0)); - - var dataSource = (ActionEndpointDataSource)CreateDataSource(mockDescriptorProvider.Object); - dataSource.AddRoute(new ConventionalRouteEntry("1", "/1/{controller}/{action}/{id?}", null, null, null)); - dataSource.AddRoute(new ConventionalRouteEntry("2", "/2/{controller}/{action}/{id?}", null, null, null)); - - dataSource.Add((b) => - { - b.Metadata.Add("Hi there"); - }); - - // Act - var endpoints = dataSource.Endpoints; - - // Assert - Assert.Collection( - endpoints.OfType().OrderBy(e => e.RoutePattern.RawText), - e => - { - Assert.Equal("/1/{controller}/{action}/{id?}", e.RoutePattern.RawText); - Assert.Same(actions[1], e.Metadata.GetMetadata()); - Assert.Equal("Hi there", e.Metadata.GetMetadata()); - }, - e => - { - Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText); - Assert.Same(actions[1], e.Metadata.GetMetadata()); - Assert.Equal("Hi there", e.Metadata.GetMetadata()); - }, - e => - { - Assert.Equal("/test", e.RoutePattern.RawText); - Assert.Same(actions[0], e.Metadata.GetMetadata()); - Assert.Equal("Hi there", e.Metadata.GetMetadata()); - }); - } - - private protected override ActionEndpointDataSourceBase CreateDataSource(IActionDescriptorCollectionProvider actions, ActionEndpointFactory endpointFactory) - { - return new ActionEndpointDataSource(actions, endpointFactory); - } - - protected override ActionDescriptor CreateActionDescriptor( - object values, - string pattern = null, - IList metadata = null) - { - var action = new ActionDescriptor(); - - foreach (var kvp in new RouteValueDictionary(values)) - { - action.RouteValues[kvp.Key] = kvp.Value?.ToString(); - } - - if (!string.IsNullOrEmpty(pattern)) - { - action.AttributeRouteInfo = new AttributeRouteInfo - { - Name = "test", - Template = pattern, - }; - } - - action.EndpointMetadata = metadata; - return action; - } - } -} diff --git a/src/Mvc/Mvc.Core/test/Routing/ActionEndpointFactoryTest.cs b/src/Mvc/Mvc.Core/test/Routing/ActionEndpointFactoryTest.cs index 61c3453168..bfe57121f6 100644 --- a/src/Mvc/Mvc.Core/test/Routing/ActionEndpointFactoryTest.cs +++ b/src/Mvc/Mvc.Core/test/Routing/ActionEndpointFactoryTest.cs @@ -269,8 +269,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing var action = CreateActionDescriptor(values); var routes = new[] { - CreateRoute(routeName: "test1", pattern: "{controller}/{action}/{id?}"), - CreateRoute(routeName: "test2", pattern: "named/{controller}/{action}/{id?}"), + CreateRoute(routeName: "test1", pattern: "{controller}/{action}/{id?}", order: 1), + CreateRoute(routeName: "test2", pattern: "named/{controller}/{action}/{id?}", order: 2), }; // Act @@ -306,7 +306,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing private RouteEndpoint CreateConventionalRoutedEndpoint(ActionDescriptor action, string template) { - return CreateConventionalRoutedEndpoint(action, new ConventionalRouteEntry(routeName: null, template, null, null, null)); + return CreateConventionalRoutedEndpoint(action, new ConventionalRouteEntry(routeName: null, template, null, null, null, order: 0)); } private RouteEndpoint CreateConventionalRoutedEndpoint(ActionDescriptor action, ConventionalRouteEntry route) @@ -340,9 +340,10 @@ namespace Microsoft.AspNetCore.Mvc.Routing string pattern, RouteValueDictionary defaults = null, IDictionary constraints = null, - RouteValueDictionary dataTokens = null) + RouteValueDictionary dataTokens = null, + int order = 0) { - return new ConventionalRouteEntry(routeName, pattern, defaults, constraints, dataTokens); + return new ConventionalRouteEntry(routeName, pattern, defaults, constraints, dataTokens, order); } private ActionDescriptor CreateActionDescriptor( diff --git a/src/Mvc/Mvc.Core/test/Routing/ControllerActionEndpointDataSourceTest.cs b/src/Mvc/Mvc.Core/test/Routing/ControllerActionEndpointDataSourceTest.cs index d58c1933da..cd8c6e6cc0 100644 --- a/src/Mvc/Mvc.Core/test/Routing/ControllerActionEndpointDataSourceTest.cs +++ b/src/Mvc/Mvc.Core/test/Routing/ControllerActionEndpointDataSourceTest.cs @@ -81,15 +81,15 @@ namespace Microsoft.AspNetCore.Mvc.Routing .Returns(new ActionDescriptorCollection(actions, 0)); var dataSource = (ControllerActionEndpointDataSource)CreateDataSource(mockDescriptorProvider.Object); - dataSource.AddRoute(new ConventionalRouteEntry("1", "/1/{controller}/{action}/{id?}", null, null, null)); - dataSource.AddRoute(new ConventionalRouteEntry("2", "/2/{controller}/{action}/{id?}", null, null, null)); + dataSource.AddRoute("1", "/1/{controller}/{action}/{id?}", null, null, null); + dataSource.AddRoute("2", "/2/{controller}/{action}/{id?}", null, null, null); // Act var endpoints = dataSource.Endpoints; // Assert Assert.Collection( - endpoints.Cast().OrderBy(e => e.RoutePattern.RawText), + endpoints.OfType().Where(e => !SupportsLinkGeneration(e)).OrderBy(e => e.RoutePattern.RawText), e => { Assert.Equal("/1/{controller}/{action}/{id?}", e.RoutePattern.RawText); @@ -99,6 +99,19 @@ namespace Microsoft.AspNetCore.Mvc.Routing { Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText); Assert.Same(actions[1], e.Metadata.GetMetadata()); + }); + + Assert.Collection( + endpoints.OfType().Where(e => SupportsLinkGeneration(e)).OrderBy(e => e.RoutePattern.RawText), + e => + { + Assert.Equal("/1/{controller}/{action}/{id?}", e.RoutePattern.RawText); + Assert.Null(e.Metadata.GetMetadata()); + }, + e => + { + Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText); + Assert.Null(e.Metadata.GetMetadata()); }, e => { @@ -139,8 +152,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing mockDescriptorProvider.Setup(m => m.ActionDescriptors).Returns(new ActionDescriptorCollection(actions, 0)); var dataSource = (ControllerActionEndpointDataSource)CreateDataSource(mockDescriptorProvider.Object); - dataSource.AddRoute(new ConventionalRouteEntry("1", "/1/{controller}/{action}/{id?}", null, null, null)); - dataSource.AddRoute(new ConventionalRouteEntry("2", "/2/{controller}/{action}/{id?}", null, null, null)); + dataSource.AddRoute("1", "/1/{controller}/{action}/{id?}", null, null, null); + dataSource.AddRoute("2", "/2/{controller}/{action}/{id?}", null, null, null); dataSource.Add((b) => { @@ -152,7 +165,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing // Assert Assert.Collection( - endpoints.OfType().OrderBy(e => e.RoutePattern.RawText), + endpoints.OfType().Where(e => !SupportsLinkGeneration(e)).OrderBy(e => e.RoutePattern.RawText), e => { Assert.Equal("/1/{controller}/{action}/{id?}", e.RoutePattern.RawText); @@ -164,6 +177,21 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText); Assert.Same(actions[1], e.Metadata.GetMetadata()); Assert.Equal("Hi there", e.Metadata.GetMetadata()); + }); + + Assert.Collection( + endpoints.OfType().Where(e => SupportsLinkGeneration(e)).OrderBy(e => e.RoutePattern.RawText), + e => + { + Assert.Equal("/1/{controller}/{action}/{id?}", e.RoutePattern.RawText); + Assert.Null(e.Metadata.GetMetadata()); + Assert.Equal("Hi there", e.Metadata.GetMetadata()); + }, + e => + { + Assert.Equal("/2/{controller}/{action}/{id?}", e.RoutePattern.RawText); + Assert.Null(e.Metadata.GetMetadata()); + Assert.Equal("Hi there", e.Metadata.GetMetadata()); }, e => { @@ -173,6 +201,11 @@ namespace Microsoft.AspNetCore.Mvc.Routing }); } + private static bool SupportsLinkGeneration(RouteEndpoint endpoint) + { + return !(endpoint.Metadata.GetMetadata()?.SuppressLinkGeneration == true); + } + private protected override ActionEndpointDataSourceBase CreateDataSource(IActionDescriptorCollectionProvider actions, ActionEndpointFactory endpointFactory) { return new ControllerActionEndpointDataSource(actions, endpointFactory); diff --git a/src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ActionEndpointDatasourceBenchmark.cs b/src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ControllerActionEndpointDatasourceBenchmark.cs similarity index 88% rename from src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ActionEndpointDatasourceBenchmark.cs rename to src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ControllerActionEndpointDatasourceBenchmark.cs index ec59bae164..c551bedb2e 100644 --- a/src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ActionEndpointDatasourceBenchmark.cs +++ b/src/Mvc/benchmarks/Microsoft.AspNetCore.Mvc.Performance/ControllerActionEndpointDatasourceBenchmark.cs @@ -14,7 +14,7 @@ using Microsoft.AspNetCore.Routing.Patterns; namespace Microsoft.AspNetCore.Mvc.Performance { - public class ActionEndpointDataSourceBenchmark + public class ControllerActionEndpointDataSourceBenchmark { private const string DefaultRoute = "{Controller=Home}/{Action=Index}/{id?}"; @@ -25,7 +25,7 @@ namespace Microsoft.AspNetCore.Mvc.Performance private MockActionDescriptorCollectionProvider _conventionalActionProvider; private MockActionDescriptorCollectionProvider _attributeActionProvider; - private List _routes; + private List<(string routeName, string pattern)> _routes; [Params(1, 100, 1000)] public int ActionCount; @@ -41,14 +41,9 @@ namespace Microsoft.AspNetCore.Mvc.Performance Enumerable.Range(0, ActionCount).Select(i => CreateAttributeRoutedAction(i)).ToList() ); - _routes = new List + _routes = new List<(string routeName, string pattern)> { - new ConventionalRouteEntry( - "Default", - DefaultRoute, - new RouteValueDictionary(), - new Dictionary(), - new RouteValueDictionary()) + ("Default", DefaultRoute) }; } @@ -67,7 +62,8 @@ namespace Microsoft.AspNetCore.Mvc.Performance var dataSource = CreateDataSource(_conventionalActionProvider); for (var i = 0; i < _routes.Count; i++) { - dataSource.AddRoute(_routes[i]); + var (routeName, pattern) = _routes[i]; + dataSource.AddRoute(routeName, pattern, defaults: null, constraints: null, dataTokens: null); } var endpoints = dataSource.Endpoints; @@ -110,9 +106,9 @@ namespace Microsoft.AspNetCore.Mvc.Performance }; } - private ActionEndpointDataSource CreateDataSource(IActionDescriptorCollectionProvider actionDescriptorCollectionProvider) + private ControllerActionEndpointDataSource CreateDataSource(IActionDescriptorCollectionProvider actionDescriptorCollectionProvider) { - var dataSource = new ActionEndpointDataSource( + var dataSource = new ControllerActionEndpointDataSource( actionDescriptorCollectionProvider, new ActionEndpointFactory(new MockRoutePatternTransformer())); diff --git a/src/Mvc/test/Mvc.FunctionalTests/LinkGeneratorTest.cs b/src/Mvc/test/Mvc.FunctionalTests/LinkGeneratorTest.cs index 1597aa98a0..8258cad06c 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/LinkGeneratorTest.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/LinkGeneratorTest.cs @@ -96,7 +96,7 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal("/Admin/LG3/SomeAction", responseContent); } - // Rejected because the calling code relies on ambient values, but doesn't pass + // This will fallback to the non-area route because the calling code relies on ambient values, but doesn't pass // the HttpContext. [Fact] public async Task GetPathByAction_FailsToGenerateLinkInsideArea() @@ -106,8 +106,8 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var responseContent = await response.Content.ReadAsStringAsync(); // Assert - Assert.Equal(HttpStatusCode.NoContent, response.StatusCode); - Assert.Equal(string.Empty, responseContent); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("/LG3/SomeAction", responseContent); } [Fact] @@ -118,8 +118,8 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests var responseContent = await response.Content.ReadAsStringAsync(); // Assert - Assert.Equal(HttpStatusCode.NoContent, response.StatusCode); - Assert.Equal(string.Empty, responseContent); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("/LG1/SomeAction", responseContent); } [Fact] @@ -229,5 +229,17 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.Equal(HttpStatusCode.OK, response.StatusCode); Assert.Equal("https://www.example.com/Admin/LGAreaPage?handler=a-handler", responseContent); } + + [Fact] + public async Task GetUriByRouteValues_CanGenerateUriToRouteWithoutMvcParameters() + { + // Act + var response = await Client.GetAsync("LG1/LinkToRouteWithNoMvcParameters?custom=17"); + var responseContent = await response.Content.ReadAsStringAsync(); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal("https://www.example.com/routewithnomvcparameters/17", responseContent); + } } } diff --git a/src/Mvc/test/Mvc.FunctionalTests/RoutingTests.cs b/src/Mvc/test/Mvc.FunctionalTests/RoutingTests.cs index a6fb8f2308..89aeda5d6c 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/RoutingTests.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/RoutingTests.cs @@ -32,48 +32,6 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.False(result); } - // Legacy routing supports linking to actions that don't exist - [Fact] - public async Task AttributeRoutedAction_InArea_StaysInArea_ActionDoesntExist() - { - // Arrange - var url = LinkFrom("http://localhost/ContosoCorp/Trains") - .To(new { action = "Contact", controller = "Home", }); - - // Act - var response = await Client.GetAsync(url); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - var body = await response.Content.ReadAsStringAsync(); - var result = JsonConvert.DeserializeObject(body); - - Assert.Equal("Rail", result.Controller); - Assert.Equal("Index", result.Action); - - Assert.Equal("/Travel/Home/Contact", result.Link); - } - - [Fact] - public async Task ConventionalRoutedAction_InArea_StaysInArea() - { - // Arrange - var url = LinkFrom("http://localhost/Travel/Flight").To(new { action = "Contact", controller = "Home", }); - - // Act - var response = await Client.GetAsync(url); - - // Assert - Assert.Equal(HttpStatusCode.OK, response.StatusCode); - var body = await response.Content.ReadAsStringAsync(); - var result = JsonConvert.DeserializeObject(body); - - Assert.Equal("Flight", result.Controller); - Assert.Equal("Index", result.Action); - - Assert.Equal("/Travel/Home/Contact", result.Link); - } - // Legacy routing returns 404 when an action does not support a HTTP method. [Fact] public override async Task AttributeRoutedAction_MultipleRouteAttributes_RouteAttributeTemplatesIgnoredForOverrideActions() diff --git a/src/Mvc/test/Mvc.FunctionalTests/RoutingTestsBase.cs b/src/Mvc/test/Mvc.FunctionalTests/RoutingTestsBase.cs index 3ff0dd20fd..f90832d402 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/RoutingTestsBase.cs +++ b/src/Mvc/test/Mvc.FunctionalTests/RoutingTestsBase.cs @@ -93,6 +93,47 @@ namespace Microsoft.AspNetCore.Mvc.FunctionalTests Assert.False(result.RouteValues.ContainsKey("page")); } + [Fact] + public async Task AttributeRoutedAction_InArea_StaysInArea_ActionDoesntExist() + { + // Arrange + var url = LinkFrom("http://localhost/ContosoCorp/Trains") + .To(new { action = "Contact", controller = "Home", }); + + // Act + var response = await Client.GetAsync(url); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Equal("Rail", result.Controller); + Assert.Equal("Index", result.Action); + + Assert.Equal("/Travel/Home/Contact", result.Link); + } + + [Fact] + public async Task ConventionalRoutedAction_InArea_StaysInArea() + { + // Arrange + var url = LinkFrom("http://localhost/Travel/Flight").To(new { action = "Contact", controller = "Home", }); + + // Act + var response = await Client.GetAsync(url); + + // Assert + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var body = await response.Content.ReadAsStringAsync(); + var result = JsonConvert.DeserializeObject(body); + + Assert.Equal("Flight", result.Controller); + Assert.Equal("Index", result.Action); + + Assert.Equal("/Travel/Home/Contact", result.Link); + } + [Fact] public abstract Task HasEndpointMatch(); diff --git a/src/Mvc/test/Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Index.Encoded.html b/src/Mvc/test/Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Index.Encoded.html index c518a8fea6..68f8bd8aba 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Index.Encoded.html +++ b/src/Mvc/test/Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Index.Encoded.html @@ -45,26 +45,26 @@ Href Order List
Non-existent Area diff --git a/src/Mvc/test/Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Index.html b/src/Mvc/test/Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Index.html index 1800bea8a1..59380d72d6 100644 --- a/src/Mvc/test/Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Index.html +++ b/src/Mvc/test/Mvc.FunctionalTests/compiler/resources/HtmlGenerationWebSite.HtmlGeneration_Home.Index.html @@ -45,26 +45,26 @@ Href Order List
Non-existent Area diff --git a/src/Mvc/test/WebSites/RoutingWebSite/Controllers/LG1Controller.cs b/src/Mvc/test/WebSites/RoutingWebSite/Controllers/LG1Controller.cs index 3cef33fa63..acf1fe2807 100644 --- a/src/Mvc/test/WebSites/RoutingWebSite/Controllers/LG1Controller.cs +++ b/src/Mvc/test/WebSites/RoutingWebSite/Controllers/LG1Controller.cs @@ -119,6 +119,15 @@ namespace RoutingWebSite values: values); } + public string LinkToRouteWithNoMvcParameters(int? custom = null) + { + return _linkGenerator.GetUriByRouteValues( + scheme: "https", + host: new HostString("www.example.com"), + routeName: "routewithnomvcparameters", + values: new { custom = custom, }); + } + private static RouteValueDictionary QueryToRouteValues(IQueryCollection query) { return new RouteValueDictionary(query.ToDictionary(kvp => kvp.Key, kvp => kvp.Value.ToString())); diff --git a/src/Mvc/test/WebSites/RoutingWebSite/StartupForLinkGenerator.cs b/src/Mvc/test/WebSites/RoutingWebSite/StartupForLinkGenerator.cs index cbb680b80e..5b6504c351 100644 --- a/src/Mvc/test/WebSites/RoutingWebSite/StartupForLinkGenerator.cs +++ b/src/Mvc/test/WebSites/RoutingWebSite/StartupForLinkGenerator.cs @@ -44,6 +44,8 @@ namespace RoutingWebSite { endpoints.MapDefaultControllerRoute(); endpoints.MapRazorPages(); + + endpoints.MapControllerRoute("routewithnomvcparameters", "/routewithnomvcparameters/{custom}"); }); } }