diff --git a/src/Microsoft.AspNet.Routing/RouteCollection.cs b/src/Microsoft.AspNet.Routing/RouteCollection.cs index 06a73cd0b6..c6f3ab1dbb 100644 --- a/src/Microsoft.AspNet.Routing/RouteCollection.cs +++ b/src/Microsoft.AspNet.Routing/RouteCollection.cs @@ -3,11 +3,13 @@ using System; using System.Collections.Generic; +using System.Diagnostics; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Routing.Logging; using Microsoft.AspNet.Routing.Logging.Internal; using Microsoft.Framework.DependencyInjection; +using Microsoft.Framework.OptionsModel; using Microsoft.Framework.Logging; namespace Microsoft.AspNet.Routing @@ -18,7 +20,9 @@ namespace Microsoft.AspNet.Routing private readonly List _unnamedRoutes = new List(); private readonly Dictionary _namedRoutes = new Dictionary(StringComparer.OrdinalIgnoreCase); + private ILogger _logger; + private RouteOptions _options; public IRouter this[int index] { @@ -94,45 +98,103 @@ namespace Microsoft.AspNet.Routing public virtual string GetVirtualPath(VirtualPathContext context) { + EnsureOptions(context.Context); + + // If we're using Best-Effort link generation then it means that we'll first look for a route where + // the route values are validated (context.IsBound == true). If we can't find a match like that, then + // we'll return the path from the first route to return one. + var useBestEffort = _options.UseBestEffortLinkGeneration; + if (!string.IsNullOrEmpty(context.RouteName)) { + var isValidated = false; + string bestPath = null; INamedRouter matchedNamedRoute; - _namedRoutes.TryGetValue(context.RouteName, out matchedNamedRoute); + if (_namedRoutes.TryGetValue(context.RouteName, out matchedNamedRoute)) + { + bestPath = matchedNamedRoute.GetVirtualPath(context); + isValidated = context.IsBound; + } - var virtualPath = matchedNamedRoute != null ? matchedNamedRoute.GetVirtualPath(context) : null; + // If we get here and context.IsBound == true, then we know we have a match, we want to keep + // iterating to see if we have multiple matches. foreach (var unnamedRoute in _unnamedRoutes) { - var tempVirtualPath = unnamedRoute.GetVirtualPath(context); - if (tempVirtualPath != null) - { - if (virtualPath != null) - { - // There was already a previous route which matched the name. - throw new InvalidOperationException( - Resources.FormatNamedRoutes_AmbiguousRoutesFound(context.RouteName)); - } + // reset because we're sharing the context + context.IsBound = false; - virtualPath = tempVirtualPath; + var path = unnamedRoute.GetVirtualPath(context); + if (path == null) + { + continue; + } + + if (bestPath != null) + { + // There was already a previous route which matched the name. + throw new InvalidOperationException( + Resources.FormatNamedRoutes_AmbiguousRoutesFound(context.RouteName)); + } + else if (context.IsBound) + { + // This is the first 'validated' match that we've found. + bestPath = path; + isValidated = true; + } + else + { + Debug.Assert(bestPath == null); + + // This is the first 'unvalidated' match that we've found. + bestPath = path; + isValidated = false; } } - return virtualPath; + if (isValidated || useBestEffort) + { + context.IsBound = isValidated; + return bestPath; + } + else + { + return null; + } } else { + string bestPath = null; for (var i = 0; i < Count; i++) { var route = this[i]; var path = route.GetVirtualPath(context); - if (path != null) + if (path == null) { + continue; + } + + if (context.IsBound) + { + // This route has validated route values, short circuit. return path; } + else if (bestPath == null) + { + // The values aren't validated, but this is the best we've seen so far + bestPath = path; + } + } + + if (useBestEffort) + { + return bestPath; + } + else + { + return null; } } - - return null; } private void EnsureLogger(HttpContext context) @@ -143,5 +205,13 @@ namespace Microsoft.AspNet.Routing _logger = factory.Create(); } } + + private void EnsureOptions(HttpContext context) + { + if (_options == null) + { + _options = context.RequestServices.GetRequiredService>().Options; + } + } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Routing/RouteOptions.cs b/src/Microsoft.AspNet.Routing/RouteOptions.cs index 3a572f2163..a17f762a93 100644 --- a/src/Microsoft.AspNet.Routing/RouteOptions.cs +++ b/src/Microsoft.AspNet.Routing/RouteOptions.cs @@ -61,5 +61,13 @@ namespace Microsoft.AspNet.Routing {"required", typeof(RequiredRouteConstraint) }, }; } + + /// + /// Gets or sets the value that enables best-effort link generation. + /// + /// If enabled, link generation will use allow link generation to succeed when the set of values provided + /// cannot be validated. + /// + public bool UseBestEffortLinkGeneration { get; set; } } } diff --git a/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs b/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs index 05d041e401..6d706061c6 100644 --- a/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs +++ b/src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs @@ -202,24 +202,18 @@ namespace Microsoft.AspNet.Routing.Template // Validate that the target can accept these values. var childContext = CreateChildVirtualPathContext(context, values.AcceptedValues); + var path = _target.GetVirtualPath(childContext); if (path != null) { // If the target generates a value then that can short circuit. - context.IsBound = true; return path; } - else if (!childContext.IsBound) - { - // The target has rejected these values. - return null; - } + // If we can produce a value go ahead and do it, the caller can check context.IsBound + // to see if the values were validated. path = _binder.BindValues(values.AcceptedValues); - if (path != null) - { - context.IsBound = true; - } + context.IsBound = childContext.IsBound; return path; } @@ -282,7 +276,7 @@ namespace Microsoft.AspNet.Routing.Template foreach (var inlineConstraint in parameter.InlineConstraints) { constraintBuilder.AddResolvedConstraint(parameter.Name, inlineConstraint.Constraint); - } + } } return constraintBuilder.Build(); diff --git a/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs b/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs index 6179818e1f..081885a8f5 100644 --- a/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs @@ -9,6 +9,7 @@ using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Routing.Logging; using Microsoft.Framework.Logging; +using Microsoft.Framework.OptionsModel; using Moq; using Xunit; @@ -231,6 +232,300 @@ namespace Microsoft.AspNet.Routing Assert.Equal("The supplied route name 'ambiguousRoute' is ambiguous and matched more than one route.", ex.Message); } + [Fact] + public void GetVirtualPath_AmbiguousRoutes_RequiresRouteValueValidation_Error() + { + // Arrange + var namedRoute = CreateNamedRoute("Ambiguous", accept: false); + + var routeCollection = new RouteCollection(); + routeCollection.Add(namedRoute); + + var innerRouteCollection = new RouteCollection(); + innerRouteCollection.Add(namedRoute); + routeCollection.Add(innerRouteCollection); + + var options = new RouteOptions() + { + UseBestEffortLinkGeneration = true, + }; + + var virtualPathContext = CreateVirtualPathContext("Ambiguous", options: options); + + // Act & Assert + var ex = Assert.Throws(() => routeCollection.GetVirtualPath(virtualPathContext)); + Assert.Equal("The supplied route name 'Ambiguous' is ambiguous and matched more than one route.", ex.Message); + } + + [Fact] + public void GetVirtualPath_NamedRoute_BestEffort_BestInTopCollection() + { + // Arrange + var bestMatch = CreateNamedRoute("Match", accept: true, matchValue: "best"); + var noMatch = CreateNamedRoute("NoMatch", accept: true, matchValue: "bad"); + + var routeCollection = new RouteCollection(); + routeCollection.Add(bestMatch); + + var innerRouteCollection = new RouteCollection(); + innerRouteCollection.Add(noMatch); + routeCollection.Add(innerRouteCollection); + + var options = new RouteOptions() + { + UseBestEffortLinkGeneration = true, + }; + + var virtualPathContext = CreateVirtualPathContext("Match", options: options); + + // Act + var path = routeCollection.GetVirtualPath(virtualPathContext); + + Assert.Equal("best", path); + } + + [Fact] + public void GetVirtualPath_NamedRoute_BestEffort_BestMatchInNestedCollection() + { + // Arrange + var bestMatch = CreateNamedRoute("NoMatch", accept: true, matchValue: "bad"); + var noMatch = CreateNamedRoute("Match", accept: true, matchValue: "best"); + + var routeCollection = new RouteCollection(); + routeCollection.Add(noMatch); + + var innerRouteCollection = new RouteCollection(); + innerRouteCollection.Add(bestMatch); + routeCollection.Add(innerRouteCollection); + + var options = new RouteOptions() + { + UseBestEffortLinkGeneration = true, + }; + + var virtualPathContext = CreateVirtualPathContext("Match", options: options); + + // Act + var path = routeCollection.GetVirtualPath(virtualPathContext); + + Assert.Equal("best", path); + } + + [Fact] + public void GetVirtualPath_NamedRoute_BestEffort_FirstRouteWins() + { + // Arrange + var bestMatch = CreateNamedRoute("Match", accept: false, matchValue: "best"); + var noMatch = CreateNamedRoute("NoMatch", accept: false, matchValue: "bad"); + + var routeCollection = new RouteCollection(); + routeCollection.Add(noMatch); + + var innerRouteCollection = new RouteCollection(); + innerRouteCollection.Add(bestMatch); + routeCollection.Add(innerRouteCollection); + + var options = new RouteOptions() + { + UseBestEffortLinkGeneration = true, + }; + + var virtualPathContext = CreateVirtualPathContext("Match", options: options); + + // Act + var path = routeCollection.GetVirtualPath(virtualPathContext); + + Assert.Equal("best", path); + } + + [Fact] + public void GetVirtualPath_BestEffort_FirstRouteWins() + { + // Arrange + var route1 = CreateRoute(accept: false, match: true, matchValue: "best"); + var route2 = CreateRoute(accept: false, match: true, matchValue: "bad"); + var route3 = CreateRoute(accept: false, match: true, matchValue: "bad"); + + var routeCollection = new RouteCollection(); + routeCollection.Add(route1.Object); + routeCollection.Add(route2.Object); + routeCollection.Add(route3.Object); + + var options = new RouteOptions() + { + UseBestEffortLinkGeneration = true, + }; + + var virtualPathContext = CreateVirtualPathContext(options: options); + + // Act + var path = routeCollection.GetVirtualPath(virtualPathContext); + + Assert.Equal("best", path); + + // All of these should be called + route1.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route2.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route3.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + } + + [Fact] + public void GetVirtualPath_NoBestEffort_NoMatch() + { + // Arrange + var route1 = CreateRoute(accept: false, match: true, matchValue: "best"); + var route2 = CreateRoute(accept: false, match: true, matchValue: "bad"); + var route3 = CreateRoute(accept: false, match: true, matchValue: "bad"); + + var routeCollection = new RouteCollection(); + routeCollection.Add(route1.Object); + routeCollection.Add(route2.Object); + routeCollection.Add(route3.Object); + + var options = new RouteOptions() + { + UseBestEffortLinkGeneration = false, + }; + + var virtualPathContext = CreateVirtualPathContext(options: options); + + // Act + var path = routeCollection.GetVirtualPath(virtualPathContext); + + Assert.Null(path); + + // All of these should be called + route1.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route2.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route3.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + } + + [Fact] + public void GetVirtualPath_BestEffort_FirstRouteWins_WithNonMatchingRoutes() + { + // Arrange + var route1 = CreateRoute(accept: false, match: false, matchValue: "bad"); + var route2 = CreateRoute(accept: false, match: true, matchValue: "best"); + var route3 = CreateRoute(accept: false, match: true, matchValue: "bad"); + + var routeCollection = new RouteCollection(); + routeCollection.Add(route1.Object); + routeCollection.Add(route2.Object); + routeCollection.Add(route3.Object); + + var options = new RouteOptions() + { + UseBestEffortLinkGeneration = true, + }; + + var virtualPathContext = CreateVirtualPathContext(options: options); + + // Act + var path = routeCollection.GetVirtualPath(virtualPathContext); + + Assert.Equal("best", path); + + // All of these should be called + route1.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route2.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route3.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + } + + [Fact] + public void GetVirtualPath_BestEffort_FirstValidatedValuesWins() + { + // Arrange + var route1 = CreateRoute(accept: false, match: true, matchValue: "bad"); + var route2 = CreateRoute(accept: false, match: true, matchValue: "bad"); + var route3 = CreateRoute(accept: true, match: true, matchValue: "best"); + + var routeCollection = new RouteCollection(); + routeCollection.Add(route1.Object); + routeCollection.Add(route2.Object); + routeCollection.Add(route3.Object); + + var options = new RouteOptions() + { + UseBestEffortLinkGeneration = true, + }; + + var virtualPathContext = CreateVirtualPathContext(options: options); + + // Act + var path = routeCollection.GetVirtualPath(virtualPathContext); + + Assert.Equal("best", path); + + // All of these should be called + route1.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route2.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route3.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + } + + [Fact] + public void GetVirtualPath_BestEffort_FirstValidatedValuesWins_ShortCircuit() + { + // Arrange + var route1 = CreateRoute(accept: false, match: true, matchValue: "bad"); + var route2 = CreateRoute(accept: true, match: true, matchValue: "best"); + var route3 = CreateRoute(accept: true, match: true, matchValue: "bad"); + + var routeCollection = new RouteCollection(); + routeCollection.Add(route1.Object); + routeCollection.Add(route2.Object); + routeCollection.Add(route3.Object); + + var options = new RouteOptions() + { + UseBestEffortLinkGeneration = true, + }; + + var virtualPathContext = CreateVirtualPathContext(options: options); + + // Act + var path = routeCollection.GetVirtualPath(virtualPathContext); + + Assert.Equal("best", path); + + route1.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route2.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route3.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Never()); + } + + [Fact] + public void GetVirtualPath_BestEffort_FirstValidatedValuesWins_Nested() + { + // Arrange + var route1 = CreateRoute(accept: false, match: true, matchValue: "bad"); + var route2 = CreateRoute(accept: false, match: true, matchValue: "bad"); + var route3 = CreateRoute(accept: true, match: true, matchValue: "best"); + + var routeCollection = new RouteCollection(); + routeCollection.Add(route1.Object); + + var innerRouteCollection = new RouteCollection(); + innerRouteCollection.Add(route2.Object); + innerRouteCollection.Add(route3.Object); + routeCollection.Add(innerRouteCollection); + + var options = new RouteOptions() + { + UseBestEffortLinkGeneration = true, + }; + + var virtualPathContext = CreateVirtualPathContext(options: options); + + // Act + var path = routeCollection.GetVirtualPath(virtualPathContext); + + Assert.Equal("best", path); + + // All of these should be called + route1.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route2.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + route3.Verify(r => r.GetVirtualPath(It.IsAny()), Times.Once()); + } + private static async Task SetUp(bool enabled, bool handled) { // Arrange @@ -256,7 +551,7 @@ namespace Microsoft.AspNet.Routing var routes = new RouteCollection(); foreach (var routeName in routeNames) { - var route1 = CreateNamedRoute(routeName); + var route1 = CreateNamedRoute(routeName, accept: true); routes.Add(route1); } @@ -265,8 +560,8 @@ namespace Microsoft.AspNet.Routing private static RouteCollection GetNestedRouteCollection(string[] routeNames) { - var rnd = new Random(); - int index = rnd.Next(0, routeNames.Length - 1); + var random = new Random(); + int index = random.Next(0, routeNames.Length - 1); var first = routeNames.Take(index).ToArray(); var second = routeNames.Skip(index).ToArray(); @@ -279,12 +574,12 @@ namespace Microsoft.AspNet.Routing rc4.Add(rc2); // Add a few unnamedRoutes. - rc1.Add(CreateRoute().Object); - rc2.Add(CreateRoute().Object); - rc3.Add(CreateRoute().Object); - rc3.Add(CreateRoute().Object); - rc4.Add(CreateRoute().Object); - rc4.Add(CreateRoute().Object); + rc1.Add(CreateRoute(accept: false).Object); + rc2.Add(CreateRoute(accept: false).Object); + rc3.Add(CreateRoute(accept: false).Object); + rc3.Add(CreateRoute(accept: false).Object); + rc4.Add(CreateRoute(accept: false).Object); + rc4.Add(CreateRoute(accept: false).Object); var routeCollection = new RouteCollection(); routeCollection.Add(rc1); @@ -293,13 +588,18 @@ namespace Microsoft.AspNet.Routing return routeCollection; } - private static INamedRouter CreateNamedRoute(string name, bool accept = false) + private static INamedRouter CreateNamedRoute(string name, bool accept = false, string matchValue = null) { + if (matchValue == null) + { + matchValue = name; + } + var target = new Mock(MockBehavior.Strict); target .Setup(e => e.GetVirtualPath(It.IsAny())) - .Callback(c => c.IsBound = accept) - .Returns(c => name) + .Callback(c => c.IsBound = accept && c.RouteName == name) + .Returns(c => c.RouteName == name ? matchValue : null) .Verifiable(); target @@ -315,36 +615,78 @@ namespace Microsoft.AspNet.Routing return target.Object; } - private static VirtualPathContext CreateVirtualPathContext(string routeName) + private static VirtualPathContext CreateVirtualPathContext( + string routeName = null, + ILoggerFactory loggerFactory = null, + RouteOptions options = null) { - return new VirtualPathContext(null, null, null, routeName); + if (loggerFactory == null) + { + loggerFactory = NullLoggerFactory.Instance; + } + + if (options == null) + { + options = new RouteOptions(); + } + + + var request = new Mock(MockBehavior.Strict); + + var optionsAccessor = new Mock>(MockBehavior.Strict); + optionsAccessor.SetupGet(o => o.Options).Returns(options); + + var context = new Mock(MockBehavior.Strict); + context.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory))) + .Returns(loggerFactory); + context.Setup(m => m.RequestServices.GetService(typeof(IOptions))) + .Returns(optionsAccessor.Object); + context.SetupGet(c => c.Request).Returns(request.Object); + + return new VirtualPathContext(context.Object, null, null, routeName); } - private static RouteContext CreateRouteContext(string requestPath, ILoggerFactory factory = null) + private static RouteContext CreateRouteContext( + string requestPath, + ILoggerFactory loggerFactory = null, + RouteOptions options = null) { - if (factory == null) + if (loggerFactory == null) { - factory = NullLoggerFactory.Instance; + loggerFactory = NullLoggerFactory.Instance; + } + + if (options == null) + { + options = new RouteOptions(); } var request = new Mock(MockBehavior.Strict); request.SetupGet(r => r.Path).Returns(new PathString(requestPath)); + var optionsAccessor = new Mock>(MockBehavior.Strict); + optionsAccessor.SetupGet(o => o.Options).Returns(options); + var context = new Mock(MockBehavior.Strict); context.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory))) - .Returns(factory); + .Returns(loggerFactory); + context.Setup(m => m.RequestServices.GetService(typeof(IOptions))) + .Returns(optionsAccessor.Object); context.SetupGet(c => c.Request).Returns(request.Object); return new RouteContext(context.Object); } - private static Mock CreateRoute(bool accept = true) + private static Mock CreateRoute( + bool accept = true, + bool match = false, + string matchValue = "value") { var target = new Mock(MockBehavior.Strict); target .Setup(e => e.GetVirtualPath(It.IsAny())) .Callback(c => c.IsBound = accept) - .Returns(null) + .Returns(accept || match ? matchValue : null) .Verifiable(); target diff --git a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs index 6309dcab39..1c5ea0db77 100644 --- a/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs +++ b/test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs @@ -971,7 +971,7 @@ namespace Microsoft.AspNet.Routing.Template } [Fact] - public void GetVirtualPath_RejectedByHandler() + public void GetVirtualPath_ValuesRejectedByHandler_StillGeneratesPath() { // Arrange var route = CreateRoute("{controller}", accept: false); @@ -982,7 +982,7 @@ namespace Microsoft.AspNet.Routing.Template // Assert Assert.False(context.IsBound); - Assert.Null(path); + Assert.Equal("Home", path); } [Fact] @@ -1026,7 +1026,7 @@ namespace Microsoft.AspNet.Routing.Template // Arrange var context = CreateVirtualPathContext(new { p1 = "hello", p2 = "1234" }); - TemplateRoute r = CreateRoute( + var r = CreateRoute( "{p1}/{p2}", new { p2 = "catchall" }, true,