huahaofeng
/
aspnetcore
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Add support for best-effort link-generation 

This change adds a feature needed for aspnet/Mvc#302

There's a new option in routing that allows link-generation to proceed
when the route values cannot be validated. The key scenario for this is
during development of an MVC site. Routing will refuse to generate a link
to actions which don't exists, this is a breaking change from the MVC5
behavior. Setting UseBestEffortLinkGeneration will allow routing to return
a value even when we can't match the action.

This option will remain off by default - setting this to on will impact
link-generation in a bunch of scenarios involving areas where we've
improved the logic for MVC6. If you're considering leaving this on outside
of development scenarios, then make sure to be as explicit with route values
as possible (don't rely on ambient values).

Functional tests to follow up in the MVC repo.
This commit is contained in:
Ryan Nowak 2015-02-09 19:21:16 -08:00
parent 4e5fc2e2dd
commit 9ee946073a
5 changed files with 464 additions and 50 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

102
src/Microsoft.AspNet.Routing/RouteCollection.cs
View File

@ -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<IRouter> _unnamedRoutes = new List<IRouter>();
private readonly Dictionary<string, INamedRouter> _namedRoutes =
new Dictionary<string, INamedRouter>(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<RouteCollection>();
}
}
private void EnsureOptions(HttpContext context)
{
if (_options == null)
{
_options = context.RequestServices.GetRequiredService<IOptions<RouteOptions>>().Options;
}
}
}
}

8
src/Microsoft.AspNet.Routing/RouteOptions.cs
View File

@ -61,5 +61,13 @@ namespace Microsoft.AspNet.Routing
{"required", typeof(RequiredRouteConstraint) },
};
}
/// <summary>
/// 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.
/// </summary>
public bool UseBestEffortLinkGeneration { get; set; }
}
}

16
src/Microsoft.AspNet.Routing/Template/TemplateRoute.cs
View File

@ -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();

382
test/Microsoft.AspNet.Routing.Tests/RouteCollectionTest.cs
View File

@ -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<InvalidOperationException>(() => 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<VirtualPathContext>()), Times.Once());
route2.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), Times.Once());
route3.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), 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<VirtualPathContext>()), Times.Once());
route2.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), Times.Once());
route3.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), 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<VirtualPathContext>()), Times.Once());
route2.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), Times.Once());
route3.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), 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<VirtualPathContext>()), Times.Once());
route2.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), Times.Once());
route3.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), 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<VirtualPathContext>()), Times.Once());
route2.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), Times.Once());
route3.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), 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<VirtualPathContext>()), Times.Once());
route2.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), Times.Once());
route3.Verify(r => r.GetVirtualPath(It.IsAny<VirtualPathContext>()), Times.Once());
}
private static async Task<TestSink> 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<INamedRouter>(MockBehavior.Strict);
target
.Setup(e => e.GetVirtualPath(It.IsAny<VirtualPathContext>()))
.Callback<VirtualPathContext>(c => c.IsBound = accept)
.Returns<VirtualPathContext>(c => name)
.Callback<VirtualPathContext>(c => c.IsBound = accept && c.RouteName == name)
.Returns<VirtualPathContext>(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<HttpRequest>(MockBehavior.Strict);
var optionsAccessor = new Mock<IOptions<RouteOptions>>(MockBehavior.Strict);
optionsAccessor.SetupGet(o => o.Options).Returns(options);
var context = new Mock<HttpContext>(MockBehavior.Strict);
context.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory)))
.Returns(loggerFactory);
context.Setup(m => m.RequestServices.GetService(typeof(IOptions<RouteOptions>)))
.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<HttpRequest>(MockBehavior.Strict);
request.SetupGet(r => r.Path).Returns(new PathString(requestPath));
var optionsAccessor = new Mock<IOptions<RouteOptions>>(MockBehavior.Strict);
optionsAccessor.SetupGet(o => o.Options).Returns(options);
var context = new Mock<HttpContext>(MockBehavior.Strict);
context.Setup(m => m.RequestServices.GetService(typeof(ILoggerFactory)))
.Returns(factory);
.Returns(loggerFactory);
context.Setup(m => m.RequestServices.GetService(typeof(IOptions<RouteOptions>)))
.Returns(optionsAccessor.Object);
context.SetupGet(c => c.Request).Returns(request.Object);
return new RouteContext(context.Object);
}
private static Mock<IRouter> CreateRoute(bool accept = true)
private static Mock<IRouter> CreateRoute(
bool accept = true,
bool match = false,
string matchValue = "value")
{
var target = new Mock<IRouter>(MockBehavior.Strict);
target
.Setup(e => e.GetVirtualPath(It.IsAny<VirtualPathContext>()))
.Callback<VirtualPathContext>(c => c.IsBound = accept)
.Returns<VirtualPathContext>(null)
.Returns(accept || match ? matchValue : null)
.Verifiable();
target

6
test/Microsoft.AspNet.Routing.Tests/Template/TemplateRouteTest.cs
View File

@ -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,