Fix for #1913 - Improve attribute route link generation for areas

Attribute route link generation will now have a slight preference for
entries that can use ambient values (vs ignoring an ambient value). This
means that areas will be more 'sticky' with regard to link generation
without the need to specify a better Order.
This commit is contained in:
Ryan Nowak 2015-02-10 15:55:54 -08:00
parent 1721d90065
commit 7cb6c1065c
6 changed files with 76 additions and 37 deletions

View File

@ -21,11 +21,11 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
new AttributeRouteLinkGenerationEntryClassifier());
}
public List<AttributeRouteLinkGenerationEntry> GetMatches(VirtualPathContext context)
public List<LinkGenerationMatch> GetMatches(VirtualPathContext context)
{
var results = new List<AttributeRouteLinkGenerationEntry>();
Walk(results, context, _root);
results.Sort(AttributeRouteLinkGenerationEntryComparer.Instance);
var results = new List<LinkGenerationMatch>();
Walk(results, context, _root, isFallbackPath: false);
results.Sort(LinkGenerationMatchComparer.Instance);
return results;
}
@ -55,15 +55,16 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
//
// The decision tree uses a tree data structure to execute these rules across all candidates at once.
private void Walk(
List<AttributeRouteLinkGenerationEntry> results,
List<LinkGenerationMatch> results,
VirtualPathContext context,
DecisionTreeNode<AttributeRouteLinkGenerationEntry> node)
DecisionTreeNode<AttributeRouteLinkGenerationEntry> node,
bool isFallbackPath)
{
// Any entries in node.Matches have had all their required values satisfied, so add them
// to the results.
for (var i = 0; i < node.Matches.Count; i++)
{
results.Add(node.Matches[i]);
results.Add(new LinkGenerationMatch(node.Matches[i], isFallbackPath));
}
for (var i = 0; i < node.Criteria.Count; i++)
@ -77,26 +78,27 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
DecisionTreeNode<AttributeRouteLinkGenerationEntry> branch;
if (criterion.Branches.TryGetValue(value ?? string.Empty, out branch))
{
Walk(results, context, branch);
Walk(results, context, branch, isFallbackPath);
}
}
else
{
// If a value wasn't explicitly supplied, match BOTH the ambient value and the empty value
// if an ambient value was supplied.
// if an ambient value was supplied. The path explored with the empty value is considered
// the fallback path.
DecisionTreeNode<AttributeRouteLinkGenerationEntry> branch;
if (context.AmbientValues.TryGetValue(key, out value) &&
!criterion.Branches.Comparer.Equals(value, string.Empty))
{
if (criterion.Branches.TryGetValue(value, out branch))
{
Walk(results, context, branch);
Walk(results, context, branch, isFallbackPath);
}
}
if (criterion.Branches.TryGetValue(string.Empty, out branch))
{
Walk(results, context, branch);
Walk(results, context, branch, isFallbackPath: true);
}
}
}
@ -123,24 +125,31 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
}
}
private class AttributeRouteLinkGenerationEntryComparer : IComparer<AttributeRouteLinkGenerationEntry>
private class LinkGenerationMatchComparer : IComparer<LinkGenerationMatch>
{
public static readonly AttributeRouteLinkGenerationEntryComparer Instance =
new AttributeRouteLinkGenerationEntryComparer();
public static readonly LinkGenerationMatchComparer Instance = new LinkGenerationMatchComparer();
public int Compare(AttributeRouteLinkGenerationEntry x, AttributeRouteLinkGenerationEntry y)
public int Compare(LinkGenerationMatch x, LinkGenerationMatch y)
{
if (x.Order != y.Order)
// For these comparisons lower is better.
if (x.Entry.Order != y.Entry.Order)
{
return x.Order.CompareTo(y.Order);
return x.Entry.Order.CompareTo(y.Entry.Order);
}
if (x.Precedence != y.Precedence)
if (x.IsFallbackMatch != y.IsFallbackMatch)
{
return x.Precedence.CompareTo(y.Precedence);
// A fallback match is worse than a non-fallback
return x.IsFallbackMatch.CompareTo(y.IsFallbackMatch);
}
return StringComparer.Ordinal.Compare(x.TemplateText, y.TemplateText);
if (x.Entry.Precedence != y.Entry.Precedence)
{
return x.Entry.Precedence.CompareTo(y.Entry.Precedence);
}
return StringComparer.Ordinal.Compare(x.Entry.TemplateText, y.Entry.TemplateText);
}
}
}

View File

@ -0,0 +1,23 @@
// Copyright (c) Microsoft Open Technologies, Inc. All rights reserved.
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using Microsoft.AspNet.Mvc.Routing;
namespace Microsoft.AspNet.Mvc.Internal.Routing
{
public struct LinkGenerationMatch
{
private readonly bool _isFallbackMatch;
private readonly AttributeRouteLinkGenerationEntry _entry;
public LinkGenerationMatch(AttributeRouteLinkGenerationEntry entry, bool isFallbackMatch)
{
_entry = entry;
_isFallbackMatch = isFallbackMatch;
}
public AttributeRouteLinkGenerationEntry Entry { get { return _entry; } }
public bool IsFallbackMatch { get { return _isFallbackMatch; } }
}
}

View File

@ -38,8 +38,8 @@ namespace Microsoft.AspNet.Mvc.Routing
_routeLogger = loggerFactory.Create<InnerAttributeRoute>();
_constraintLogger = loggerFactory.Create(typeof(RouteConstraintMatcher).FullName);
// Force creation of the route to report issues on startup.
GetInnerRoute();
// Force creation of the route to report issues on startup.
GetInnerRoute();
}
/// <inheritdoc />
@ -305,4 +305,4 @@ namespace Microsoft.AspNet.Mvc.Routing
public string Name { get; set; }
}
}
}
}

View File

@ -154,9 +154,9 @@ namespace Microsoft.AspNet.Mvc.Routing
// order. We just need to iterate them and use the first one that can generate a link.
var matches = _linkGenerationTree.GetMatches(context);
foreach (var entry in matches)
foreach (var match in matches)
{
var path = GenerateLink(context, entry);
var path = GenerateLink(context, match.Entry);
if (path != null)
{
context.IsBound = true;

View File

@ -2,6 +2,7 @@
// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information.
using System.Collections.Generic;
using System.Linq;
using Microsoft.AspNet.Http.Core;
using Microsoft.AspNet.Mvc.Routing;
using Microsoft.AspNet.Routing;
@ -28,7 +29,7 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
var matches = tree.GetMatches(context);
// Assert
Assert.Same(entry, Assert.Single(matches));
Assert.Same(entry, Assert.Single(matches).Entry);
}
[Fact]
@ -48,7 +49,7 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
var matches = tree.GetMatches(context);
// Assert
Assert.Same(entry, Assert.Single(matches));
Assert.Same(entry, Assert.Single(matches).Entry);
}
[Fact]
@ -68,7 +69,9 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
var matches = tree.GetMatches(context);
// Assert
Assert.Same(entry, Assert.Single(matches));
var match = Assert.Single(matches);
Assert.Same(entry, match.Entry);
Assert.False(match.IsFallbackMatch);
}
[Fact]
@ -90,7 +93,9 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
var matches = tree.GetMatches(context);
// Assert
Assert.Same(entry, Assert.Single(matches));
var match = Assert.Single(matches);
Assert.Same(entry, match.Entry);
Assert.False(match.IsFallbackMatch);
}
[Fact]
@ -112,7 +117,9 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
var matches = tree.GetMatches(context);
// Assert
Assert.Same(entry, Assert.Single(matches));
var match = Assert.Single(matches);
Assert.Same(entry, match.Entry);
Assert.True(match.IsFallbackMatch);
}
[Fact]
@ -179,7 +186,7 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
var matches = tree.GetMatches(context);
// Assert
Assert.Same(entry1, Assert.Single(matches));
Assert.Same(entry1, Assert.Single(matches).Entry);
}
[Fact]
@ -202,7 +209,7 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
ambientValues: new { controller = "Store", action = "Buy" });
// Act
var matches = tree.GetMatches(context);
var matches = tree.GetMatches(context).Select(m => m.Entry).ToList();
// Assert
Assert.Equal(entries, matches);
@ -226,7 +233,7 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
var context = CreateContext(new { controller = "Store", action = "Buy", slug = "1234" });
// Act
var matches = tree.GetMatches(context);
var matches = tree.GetMatches(context).Select(m => m.Entry).ToList();
// Assert
Assert.Equal(entries, matches);
@ -253,7 +260,7 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
var context = CreateContext(new { controller = "Store", action = "Buy" });
// Act
var matches = tree.GetMatches(context);
var matches = tree.GetMatches(context).Select(m => m.Entry).ToList();
// Assert
Assert.Equal(entries, matches);
@ -279,7 +286,7 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
var context = CreateContext(new { controller = "Store", action = "Buy" });
// Act
var matches = tree.GetMatches(context);
var matches = tree.GetMatches(context).Select(m => m.Entry).ToList();
// Assert
Assert.Equal(entries, matches);
@ -305,7 +312,7 @@ namespace Microsoft.AspNet.Mvc.Internal.Routing
var context = CreateContext(new { controller = "Store", action = "Buy" });
// Act
var matches = tree.GetMatches(context);
var matches = tree.GetMatches(context).Select(m => m.Entry).ToList();
// Assert
Assert.Equal(entries, matches);

View File

@ -107,4 +107,4 @@ namespace Microsoft.AspNet.Mvc.Routing
}
}
}
#endif
#endif