diff --git a/src/Http/Routing/src/RouteCollection.cs b/src/Http/Routing/src/RouteCollection.cs index deeb71c6334c8e0971ca14f31aa9a6ee2a7401ba..55e21d42253f31b132824e573b4a1ca2f613de51 100644 --- a/src/Http/Routing/src/RouteCollection.cs +++ b/src/Http/Routing/src/RouteCollection.cs @@ -15,7 +15,7 @@ using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Routing { /// <summary> - /// Supports managing a collection fo multiple routes. + /// Supports managing a collection for multiple routes. /// </summary> public class RouteCollection : IRouteCollection { diff --git a/src/Http/Routing/src/RouteValuesAddressScheme.cs b/src/Http/Routing/src/RouteValuesAddressScheme.cs index 640129c01b07711e7b0dcf650af645069d9668e0..e9e7db70feecc44adae0911fb73e00e9c20a4e24 100644 --- a/src/Http/Routing/src/RouteValuesAddressScheme.cs +++ b/src/Http/Routing/src/RouteValuesAddressScheme.cs @@ -73,7 +73,7 @@ namespace Microsoft.AspNetCore.Routing private StateEntry Initialize(IReadOnlyList<Endpoint> endpoints) { - var allOutboundMatches = new List<OutboundMatch>(); + var matchesWithRequiredValues = new List<OutboundMatch>(); var namedOutboundMatchResults = new Dictionary<string, List<OutboundMatchResult>>(StringComparer.OrdinalIgnoreCase); // Decision tree is built using the 'required values' of actions. @@ -118,7 +118,15 @@ namespace Microsoft.AspNetCore.Routing metadata?.RouteName); var outboundMatch = new OutboundMatch() { Entry = entry }; - allOutboundMatches.Add(outboundMatch); + + if (routeEndpoint.RoutePattern.RequiredValues.Count > 0) + { + // Entries with a route name but no required values can only be matched by name. + // Otherwise, these endpoints will match any attempt at action link generation. + // Entries with neither a route name nor required values have already been skipped above. + // See https://github.com/dotnet/aspnetcore/issues/35592 + matchesWithRequiredValues.Add(outboundMatch); + } if (string.IsNullOrEmpty(entry.RouteName)) { @@ -134,8 +142,8 @@ namespace Microsoft.AspNetCore.Routing } return new StateEntry( - allOutboundMatches, - new LinkGenerationDecisionTree(allOutboundMatches), + matchesWithRequiredValues, + new LinkGenerationDecisionTree(matchesWithRequiredValues), namedOutboundMatchResults); } @@ -166,16 +174,16 @@ namespace Microsoft.AspNetCore.Routing internal class StateEntry { // For testing - public readonly List<OutboundMatch> AllMatches; + public readonly List<OutboundMatch> MatchesWithRequiredValues; public readonly LinkGenerationDecisionTree AllMatchesLinkGenerationTree; public readonly Dictionary<string, List<OutboundMatchResult>> NamedMatches; public StateEntry( - List<OutboundMatch> allMatches, + List<OutboundMatch> matchesWithRequiredValues, LinkGenerationDecisionTree allMatchesLinkGenerationTree, Dictionary<string, List<OutboundMatchResult>> namedMatches) { - AllMatches = allMatches; + MatchesWithRequiredValues = matchesWithRequiredValues; AllMatchesLinkGenerationTree = allMatchesLinkGenerationTree; NamedMatches = namedMatches; } diff --git a/src/Http/Routing/test/UnitTests/RouteValuesAddressSchemeTest.cs b/src/Http/Routing/test/UnitTests/RouteValuesAddressSchemeTest.cs index b7127935f41ca11b50e46c23a8b5e78a388a8e80..cf1f1ad259cf7c5fe8ca30a0fc80f101965858d7 100644 --- a/src/Http/Routing/test/UnitTests/RouteValuesAddressSchemeTest.cs +++ b/src/Http/Routing/test/UnitTests/RouteValuesAddressSchemeTest.cs @@ -1,12 +1,9 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections.Generic; -using System.Linq; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.AspNetCore.Routing.TestObjects; -using Xunit; namespace Microsoft.AspNetCore.Routing { @@ -23,9 +20,8 @@ namespace Microsoft.AspNetCore.Routing var addressScheme = CreateAddressScheme(endpoint1, endpoint2); // Assert - Assert.NotNull(addressScheme.State.AllMatches); - Assert.Equal(2, addressScheme.State.AllMatches.Count()); - Assert.NotNull(addressScheme.State.NamedMatches); + var allMatches = GetMatchesWithRequiredValuesPlusNamedMatches(addressScheme); + Assert.Equal(2, allMatches.Count); Assert.True(addressScheme.State.NamedMatches.TryGetValue("named", out var namedMatches)); var namedMatch = Assert.Single(namedMatches); var actual = Assert.IsType<RouteEndpoint>(namedMatch.Match.Entry.Data); @@ -44,9 +40,8 @@ namespace Microsoft.AspNetCore.Routing var addressScheme = CreateAddressScheme(endpoint1, endpoint2, endpoint3); // Assert - Assert.NotNull(addressScheme.State.AllMatches); - Assert.Equal(3, addressScheme.State.AllMatches.Count()); - Assert.NotNull(addressScheme.State.NamedMatches); + var allMatches = GetMatchesWithRequiredValuesPlusNamedMatches(addressScheme); + Assert.Equal(3, allMatches.Count); Assert.True(addressScheme.State.NamedMatches.TryGetValue("named", out var namedMatches)); Assert.Equal(2, namedMatches.Count); Assert.Same(endpoint2, Assert.IsType<RouteEndpoint>(namedMatches[0].Match.Entry.Data)); @@ -65,9 +60,8 @@ namespace Microsoft.AspNetCore.Routing var addressScheme = CreateAddressScheme(endpoint1, endpoint2, endpoint3); // Assert - Assert.NotNull(addressScheme.State.AllMatches); - Assert.Equal(3, addressScheme.State.AllMatches.Count()); - Assert.NotNull(addressScheme.State.NamedMatches); + var allMatches = GetMatchesWithRequiredValuesPlusNamedMatches(addressScheme); + Assert.Equal(3, allMatches.Count); Assert.True(addressScheme.State.NamedMatches.TryGetValue("named", out var namedMatches)); Assert.Equal(2, namedMatches.Count); Assert.Same(endpoint2, Assert.IsType<RouteEndpoint>(namedMatches[0].Match.Entry.Data)); @@ -86,8 +80,11 @@ namespace Microsoft.AspNetCore.Routing // Assert 1 var state = addressScheme.State; - Assert.NotNull(state.AllMatches); - var match = Assert.Single(state.AllMatches); + var allMatches = GetMatchesWithRequiredValuesPlusNamedMatches(addressScheme); + + Assert.NotEmpty(allMatches); + + var match = Assert.Single(allMatches); var actual = Assert.IsType<RouteEndpoint>(match.Entry.Data); Assert.Same(endpoint1, actual); @@ -124,9 +121,11 @@ namespace Microsoft.AspNetCore.Routing Assert.NotSame(state, addressScheme.State); state = addressScheme.State; - Assert.NotNull(state.AllMatches); + allMatches = GetMatchesWithRequiredValuesPlusNamedMatches(addressScheme); + + Assert.NotEmpty(allMatches); Assert.Collection( - state.AllMatches, + allMatches, (m) => { actual = Assert.IsType<RouteEndpoint>(m.Entry.Data); @@ -335,19 +334,66 @@ namespace Microsoft.AspNetCore.Routing Assert.Same(expected, actual); } + [Fact] + public void GetOutboundMatches_Includes_SameEndpointInNamedMatchesAndMatchesWithRequiredValues() + { + // Arrange + var endpoint = CreateEndpoint( + "api/orders/{id}", + defaults: new { controller = "Orders", action = "GetById" }, + metadataRequiredValues: new { controller = "Orders", action = "GetById" }, + routeName: "a"); + + // Act + var addressScheme = CreateAddressScheme(endpoint); + + // Assert + var matchWithRequiredValue = Assert.Single(addressScheme.State.MatchesWithRequiredValues); + var namedMatches = Assert.Single(addressScheme.State.NamedMatches).Value; + var namedMatch = Assert.Single(namedMatches).Match; + + Assert.Same(endpoint, matchWithRequiredValue.Entry.Data); + Assert.Same(endpoint, namedMatch.Entry.Data); + } + + // Regression test for https://github.com/dotnet/aspnetcore/issues/35592 + [Fact] + public void GetOutboundMatches_DoesNotInclude_EndpointsWithoutRequiredValuesInMatchesWithRequiredValues() + { + // Arrange + var endpoint = CreateEndpoint( + "api/orders/{id}", + defaults: new { controller = "Orders", action = "GetById" }, + routeName: "a"); + + // Act + var addressScheme = CreateAddressScheme(endpoint); + + // Assert + Assert.Empty(addressScheme.State.MatchesWithRequiredValues); + + var namedMatches = Assert.Single(addressScheme.State.NamedMatches).Value; + var namedMatch = Assert.Single(namedMatches).Match; + Assert.Same(endpoint, namedMatch.Entry.Data); + } + [Fact] public void GetOutboundMatches_DoesNotInclude_EndpointsWithSuppressLinkGenerationMetadata() { // Arrange var endpoint = CreateEndpoint( - "/a", + "api/orders/{id}", + defaults: new { controller = "Orders", action = "GetById" }, + metadataRequiredValues: new { controller = "Orders", action = "GetById" }, + routeName: "a", metadataCollection: new EndpointMetadataCollection(new[] { new SuppressLinkGenerationMetadata() })); // Act var addressScheme = CreateAddressScheme(endpoint); // Assert - Assert.Empty(addressScheme.State.AllMatches); + var allMatches = GetMatchesWithRequiredValuesPlusNamedMatches(addressScheme); + Assert.Empty(allMatches); } [Fact] @@ -356,13 +402,14 @@ namespace Microsoft.AspNetCore.Routing // Arrange var endpoint = EndpointFactory.CreateRouteEndpoint( "/a", - metadata: new object[] { new SuppressLinkGenerationMetadata(), new EncourageLinkGenerationMetadata(), new RouteNameMetadata(string.Empty), }); + metadata: new object[] { new SuppressLinkGenerationMetadata(), new EncourageLinkGenerationMetadata(), new RouteNameMetadata("a"), }); // Act var addressScheme = CreateAddressScheme(endpoint); // Assert - Assert.Same(endpoint, Assert.Single(addressScheme.State.AllMatches).Entry.Data); + var allMatches = GetMatchesWithRequiredValuesPlusNamedMatches(addressScheme); + Assert.Same(endpoint, Assert.Single(allMatches).Entry.Data); } private RouteValuesAddressScheme CreateAddressScheme(params Endpoint[] endpoints) @@ -401,6 +448,18 @@ namespace Microsoft.AspNetCore.Routing null); } + private static List<Tree.OutboundMatch> GetMatchesWithRequiredValuesPlusNamedMatches(RouteValuesAddressScheme routeValuesAddressScheme) + { + var state = routeValuesAddressScheme.State; + + Assert.NotNull(state.MatchesWithRequiredValues); + Assert.NotNull(state.NamedMatches); + + var namedMatches = state.NamedMatches.Aggregate(Enumerable.Empty<Tree.OutboundMatch>(), + (acc, kvp) => acc.Concat(kvp.Value.Select(matchResult => matchResult.Match))); + return state.MatchesWithRequiredValues.Concat(namedMatches).ToList(); + } + private class EncourageLinkGenerationMetadata : ISuppressLinkGenerationMetadata { public bool SuppressLinkGeneration => false; diff --git a/src/Mvc/Mvc.Core/test/Routing/EndpointRoutingUrlHelperTest.cs b/src/Mvc/Mvc.Core/test/Routing/EndpointRoutingUrlHelperTest.cs index 9043f7b18dfdb9dcf1341715535121af93889184..7290fb8be674abbfd7bfe7ba2a157af14f1614de 100644 --- a/src/Mvc/Mvc.Core/test/Routing/EndpointRoutingUrlHelperTest.cs +++ b/src/Mvc/Mvc.Core/test/Routing/EndpointRoutingUrlHelperTest.cs @@ -1,18 +1,11 @@ -// Licensed to the .NET Foundation under one or more agreements. +// Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Collections.Generic; -using System.Linq; -using System.Threading.Tasks; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Routing; -using Microsoft.AspNetCore.Routing.Matching; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; -using Xunit; namespace Microsoft.AspNetCore.Mvc.Routing { @@ -145,7 +138,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing protected override IUrlHelper CreateUrlHelper(ActionContext actionContext) { var httpContext = actionContext.HttpContext; - httpContext.SetEndpoint(new Endpoint(context => Task.CompletedTask, EndpointMetadataCollection.Empty, null)); + httpContext.SetEndpoint(new Endpoint(context => Task.CompletedTask, EndpointMetadataCollection.Empty, null)); var urlHelperFactory = httpContext.RequestServices.GetRequiredService<IUrlHelperFactory>(); var urlHelper = urlHelperFactory.GetUrlHelper(actionContext); @@ -164,9 +157,10 @@ namespace Microsoft.AspNetCore.Mvc.Routing string protocol, string routeName, string template, - object defaults) + object defaults, + object requiredValues) { - var endpoint = CreateEndpoint(template, new RouteValueDictionary(defaults), routeName: routeName); + var endpoint = CreateEndpoint(template, new RouteValueDictionary(defaults), requiredValues, routeName: routeName); var services = CreateServices(new[] { endpoint }); var httpContext = CreateHttpContext(services, appRoot: "", host: null, protocol: null); var actionContext = CreateActionContext(httpContext); diff --git a/src/Mvc/Mvc.Core/test/Routing/UrlHelperTest.cs b/src/Mvc/Mvc.Core/test/Routing/UrlHelperTest.cs index cf046a749d79a386fd4239991b51f98711268313..93b883e42887f73d689ba6408f86aa2c1944c888 100644 --- a/src/Mvc/Mvc.Core/test/Routing/UrlHelperTest.cs +++ b/src/Mvc/Mvc.Core/test/Routing/UrlHelperTest.cs @@ -1,14 +1,10 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System; -using System.Text; -using System.Threading.Tasks; using Microsoft.AspNetCore.Builder; using Microsoft.AspNetCore.Routing; using Microsoft.Extensions.DependencyInjection; using Moq; -using Xunit; namespace Microsoft.AspNetCore.Mvc.Routing { @@ -68,7 +64,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing string protocol, string routeName, string template, - object defaults) + object defaults, + object requiredValues) { var services = CreateServices(); var routeBuilder = CreateRouteBuilder(services); @@ -152,4 +149,4 @@ namespace Microsoft.AspNetCore.Mvc.Routing } } } -} \ No newline at end of file +} diff --git a/src/Mvc/Mvc.Core/test/Routing/UrlHelperTestBase.cs b/src/Mvc/Mvc.Core/test/Routing/UrlHelperTestBase.cs index 7deacc13777b85baabeb3a518da1191b7f592f30..74bc0740efea9546c9deffffe6e2b340cf6b079d 100644 --- a/src/Mvc/Mvc.Core/test/Routing/UrlHelperTestBase.cs +++ b/src/Mvc/Mvc.Core/test/Routing/UrlHelperTestBase.cs @@ -8,6 +8,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc.Abstractions; using Microsoft.AspNetCore.Routing; +using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.DependencyInjection; using Moq; using Xunit; @@ -769,6 +770,57 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.Equal("http://localhost/app/home/newaction/someid", url); } + [Fact] + public void LinkWithNullRouteNameGivenExtraEndpointWithNoRouteNameAndNoRequiredValues_ReturnsExpectedResult() + { + // Arrange + var urlHelper = CreateUrlHelperWithDefaultRoutes( + "/app", + host: null, + protocol: null, + routeName: null, + template: "any/url"); + + // Act + var url = urlHelper.Link( + null, + new + { + Action = "newaction", + Controller = "home", + id = "someid" + }); + + // Assert + Assert.Equal("http://localhost/app/home/newaction/someid", url); + } + + // Regression test for https://github.com/dotnet/aspnetcore/issues/35592 + [Fact] + public void LinkWithNullRouteNameGivenExtraEndpointWithRouteNameAndNoRequiredValues_ReturnsExpectedResult() + { + // Arrange + var urlHelper = CreateUrlHelperWithDefaultRoutes( + "/app", + host: null, + protocol: null, + routeName: "MyRouteName", + template: "any/url"); + + // Act + var url = urlHelper.Link( + null, + new + { + Action = "newaction", + Controller = "home", + id = "someid" + }); + + // Assert + Assert.Equal("http://localhost/app/home/newaction/someid", url); + } + [Fact] public void RouteUrlWithRouteNameAndDefaults() { @@ -895,7 +947,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.Same(urlHelper, actionContext.HttpContext.Items[typeof(IUrlHelper)] as IUrlHelper); } - // Regression test for aspnet/Mvc#2859 + // Regression test for https://github.com/aspnet/Mvc/issues/2859 [Fact] public void Action_RouteValueInvalidation_DoesNotAffectActionAndController() { @@ -904,15 +956,23 @@ namespace Microsoft.AspNetCore.Mvc.Routing appRoot: "", host: null, protocol: null, - "default", - "{first}/{controller}/{action}", - new { second = "default", controller = "default", action = "default" }); + routeName: "default", + template: "{first}/{controller}/{action}", + defaults: new { second = "default", controller = "default", action = "default" }, + // Emulate ActionEndpointFactory.AddConventionalLinkGenerationRoute(). + // The "controller" and "action" keys are defined automatically by ControllerActionDescriptorBuilder.AddRouteValues(). + requiredValues: new { controller = RoutePattern.RequiredValueAny, action = RoutePattern.RequiredValueAny }); var routeData = urlHelper.ActionContext.RouteData; routeData.Values.Add("first", "a"); routeData.Values.Add("controller", "Store"); routeData.Values.Add("action", "Buy"); + urlHelper.ActionContext.HttpContext.Features.Set<IRouteValuesFeature>(new RouteValuesFeature + { + RouteValues = routeData.Values + }); + // Act // // In this test the 'first' route value has changed, meaning that *normally* the @@ -925,7 +985,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.Equal("/b/Store/Checkout", url); } - // Regression test for aspnet/Mvc#2859 + // Regression test for https://github.com/aspnet/Mvc/issues/2859 [Fact] public void Action_RouteValueInvalidation_AffectsOtherRouteValues() { @@ -934,9 +994,12 @@ namespace Microsoft.AspNetCore.Mvc.Routing appRoot: "", host: null, protocol: null, - "default", - "{first}/{second}/{controller}/{action}", - new { second = "default", controller = "default", action = "default" }); + routeName: "default", + template: "{first}/{second}/{controller}/{action}", + defaults: new { second = "default", controller = "default", action = "default" }, + // Emulate ActionEndpointFactory.AddConventionalLinkGenerationRoute(). + // The "controller" and "action" keys are defined automatically by ControllerActionDescriptorBuilder.AddRouteValues(). + requiredValues: new { controller = RoutePattern.RequiredValueAny, action = RoutePattern.RequiredValueAny }); var routeData = urlHelper.ActionContext.RouteData; routeData.Values.Add("first", "a"); @@ -944,6 +1007,11 @@ namespace Microsoft.AspNetCore.Mvc.Routing routeData.Values.Add("controller", "Store"); routeData.Values.Add("action", "Buy"); + urlHelper.ActionContext.HttpContext.Features.Set<IRouteValuesFeature>(new RouteValuesFeature + { + RouteValues = routeData.Values + }); + // Act // // In this test the 'first' route value has changed, meaning that *normally* the @@ -958,7 +1026,7 @@ namespace Microsoft.AspNetCore.Mvc.Routing Assert.Equal("/b/default/Store/Checkout", url); } - // Regression test for aspnet/Mvc#2859 + // Regression test for https://github.com/aspnet/Mvc/issues/2859 [Fact] public void Action_RouteValueInvalidation_DoesNotAffectActionAndController_ActionPassedInRouteValues() { @@ -967,15 +1035,23 @@ namespace Microsoft.AspNetCore.Mvc.Routing appRoot: "", host: null, protocol: null, - "default", - "{first}/{controller}/{action}", - new { second = "default", controller = "default", action = "default" }); + routeName: "default", + template: "{first}/{controller}/{action}", + defaults: new { second = "default", controller = "default", action = "default" }, + // Emulate ActionEndpointFactory.AddConventionalLinkGenerationRoute(). + // The "controller" and "action" keys are defined automatically by ControllerActionDescriptorBuilder.AddRouteValues(). + requiredValues: new { controller = RoutePattern.RequiredValueAny, action = RoutePattern.RequiredValueAny }); var routeData = urlHelper.ActionContext.RouteData; routeData.Values.Add("first", "a"); routeData.Values.Add("controller", "Store"); routeData.Values.Add("action", "Buy"); + urlHelper.ActionContext.HttpContext.Features.Set<IRouteValuesFeature>(new RouteValuesFeature + { + RouteValues = routeData.Values + }); + // Act // // In this test the 'first' route value has changed, meaning that *normally* the @@ -1044,7 +1120,8 @@ namespace Microsoft.AspNetCore.Mvc.Routing string protocol, string routeName, string template, - object defaults); + object defaults, + object requiredValues); protected virtual IUrlHelper CreateUrlHelper(string appRoot, string host, string protocol) {