From 9faca78a84347328355273a0412d2826730d6cb5 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Wed, 23 Jul 2014 14:44:18 -0700 Subject: [PATCH] Optimize attribute routing link generation --- .../Internal/DecisionTree/DecisionTreeNode.cs | 6 + .../Routing/LinkGenerationDecisionTree.cs | 147 ++++++++ .../Routing/AttributeRoute.cs | 44 +-- .../Routing/LinkGenerationDecisionTreeTest.cs | 331 ++++++++++++++++++ 4 files changed, 498 insertions(+), 30 deletions(-) create mode 100644 src/Microsoft.AspNet.Mvc.Core/Internal/Routing/LinkGenerationDecisionTree.cs create mode 100644 test/Microsoft.AspNet.Mvc.Core.Test/Internal/Routing/LinkGenerationDecisionTreeTest.cs diff --git a/src/Microsoft.AspNet.Mvc.Core/Internal/DecisionTree/DecisionTreeNode.cs b/src/Microsoft.AspNet.Mvc.Core/Internal/DecisionTree/DecisionTreeNode.cs index 3defcf0570..3f49b8eff4 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Internal/DecisionTree/DecisionTreeNode.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Internal/DecisionTree/DecisionTreeNode.cs @@ -5,10 +5,16 @@ using System.Collections.Generic; namespace Microsoft.AspNet.Mvc.Internal.DecisionTree { + // Data structure representing a node in a decision tree. These are created in DecisionTreeBuilder + // and walked to find a set of items matching some input criteria. public class DecisionTreeNode { + // The list of matches for the current node. This represents a set of items that have had all + // of their criteria matched if control gets to this point in the tree. public List Matches { get; set; } + // Additional criteria that further branch out from this node. Walk these to fine more items + // matching the input data. public List> Criteria { get; set; } } } \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Internal/Routing/LinkGenerationDecisionTree.cs b/src/Microsoft.AspNet.Mvc.Core/Internal/Routing/LinkGenerationDecisionTree.cs new file mode 100644 index 0000000000..d52cfdaa35 --- /dev/null +++ b/src/Microsoft.AspNet.Mvc.Core/Internal/Routing/LinkGenerationDecisionTree.cs @@ -0,0 +1,147 @@ +// 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 System; +using System.Collections.Generic; +using Microsoft.AspNet.Mvc.Internal.DecisionTree; +using Microsoft.AspNet.Mvc.Routing; +using Microsoft.AspNet.Routing; + +namespace Microsoft.AspNet.Mvc.Internal.Routing +{ + // A decision tree that matches link generation entries based on route data. + public class LinkGenerationDecisionTree + { + private readonly DecisionTreeNode _root; + + public LinkGenerationDecisionTree(IReadOnlyList entries) + { + _root = DecisionTreeBuilder.GenerateTree( + entries, + new AttributeRouteLinkGenerationEntryClassifier()); + } + + public List GetMatches(VirtualPathContext context) + { + var results = new List(); + Walk(results, context, _root); + results.Sort(AttributeRouteLinkGenerationEntryComparer.Instance); + return results; + } + + // We need to recursively walk the decision tree based on the provided route data + // (context.Values + context.AmbientValues) to find all entries that match. This process is + // virtually identical to action selection. + // + // Each entry has a collection of 'required link values' that must be satisfied. These are + // key-value pairs that make up the decision tree. + // + // A 'require link value' is considered satisfied IF: + // 1. The value in context.Values matches the required value OR + // 2. There is no value in context.Values and the value in context.AmbientValues matches OR + // 3. The required value is 'null' and there is no value in context.Values. + // + // Ex: + // entry requires { area = null, controller = Store, action = Buy } + // context.Values = { controller = Store, action = Buy } + // context.AmbientValues = { area = Help, controller = AboutStore, action = HowToBuyThings } + // + // In this case the entry is a match. The 'controller' and 'action' are both supplied by context.Values, + // and the 'area' is satisfied because there's NOT a value in context.Values. It's OK to ignore ambient + // values in link generation. + // + // If another entry existed like { area = Help, controller = Store, action = Buy }, this would also + // match. + // + // The decision tree uses a tree data structure to execute these rules across all candidates at once. + private void Walk( + List results, + VirtualPathContext context, + DecisionTreeNode node) + { + // 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]); + } + + for (var i = 0; i < node.Criteria.Count; i++) + { + var criterion = node.Criteria[i]; + var key = criterion.Key; + + object value; + if (context.Values.TryGetValue(key, out value)) + { + DecisionTreeNode branch; + if (criterion.Branches.TryGetValue(value ?? string.Empty, out branch)) + { + Walk(results, context, branch); + } + } + else + { + // If a value wasn't explicitly supplied, match BOTH the ambient value and the empty value + // if an ambient value was supplied. + DecisionTreeNode 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); + } + } + + if (criterion.Branches.TryGetValue(string.Empty, out branch)) + { + Walk(results, context, branch); + } + } + } + } + + private class AttributeRouteLinkGenerationEntryClassifier : IClassifier + { + public AttributeRouteLinkGenerationEntryClassifier() + { + ValueComparer = new RouteValueEqualityComparer(); + } + + public IEqualityComparer ValueComparer { get; private set; } + + public IDictionary GetCriteria(AttributeRouteLinkGenerationEntry item) + { + var results = new Dictionary(StringComparer.OrdinalIgnoreCase); + foreach (var kvp in item.RequiredLinkValues) + { + results.Add(kvp.Key, new DecisionCriterionValue(kvp.Value ?? string.Empty, isCatchAll: false)); + } + + return results; + } + } + + private class AttributeRouteLinkGenerationEntryComparer : IComparer + { + public static readonly AttributeRouteLinkGenerationEntryComparer Instance = + new AttributeRouteLinkGenerationEntryComparer(); + + public int Compare(AttributeRouteLinkGenerationEntry x, AttributeRouteLinkGenerationEntry y) + { + if (x.Order != y.Order) + { + return x.Order.CompareTo(y.Order); + } + + if (x.Precedence != y.Precedence) + { + return x.Precedence.CompareTo(y.Precedence); + } + + return StringComparer.Ordinal.Compare(x.TemplateText, y.TemplateText); + } + } + } +} \ No newline at end of file diff --git a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs index a71d902924..7e53483a04 100644 --- a/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs +++ b/src/Microsoft.AspNet.Mvc.Core/Routing/AttributeRoute.cs @@ -5,6 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; +using Microsoft.AspNet.Mvc.Internal.Routing; using Microsoft.AspNet.Mvc.Logging; using Microsoft.AspNet.Routing; using Microsoft.AspNet.Routing.Template; @@ -19,9 +20,9 @@ namespace Microsoft.AspNet.Mvc.Routing { private readonly IRouter _next; private readonly TemplateRoute[] _matchingRoutes; - private readonly AttributeRouteLinkGenerationEntry[] _linkGenerationEntries; private ILogger _logger; private ILogger _constraintLogger; + private readonly LinkGenerationDecisionTree _linkGenerationTree; /// /// Creates a new . @@ -29,7 +30,7 @@ namespace Microsoft.AspNet.Mvc.Routing /// The next router. Invoked when a route entry matches. /// The set of route entries. public AttributeRoute( - [NotNull] IRouter next, + [NotNull] IRouter next, [NotNull] IEnumerable matchingEntries, [NotNull] IEnumerable linkGenerationEntries, [NotNull] ILoggerFactory factory) @@ -48,11 +49,8 @@ namespace Microsoft.AspNet.Mvc.Routing .Select(e => e.Route) .ToArray(); - _linkGenerationEntries = linkGenerationEntries - .OrderBy(o => o.Order) - .ThenBy(e => e.Precedence) - .ThenBy(e => e.TemplateText, StringComparer.Ordinal) - .ToArray(); + // The decision tree will take care of ordering for these entries. + _linkGenerationTree = new LinkGenerationDecisionTree(linkGenerationEntries.ToArray()); _logger = factory.Create(); _constraintLogger = factory.Create(typeof(RouteConstraintMatcher).FullName); @@ -87,31 +85,17 @@ namespace Microsoft.AspNet.Mvc.Routing /// public string GetVirtualPath([NotNull] VirtualPathContext context) { - // To generate a link, we iterate the collection of entries (in order of precedence) and execute - // each one that matches the 'required link values' - which will typically be a value for action - // and controller. - // - // Building a proper data structure to optimize this is tracked by #741 - foreach (var entry in _linkGenerationEntries) + // The decision tree will give us back all entries that match the provided route data in the correct + // 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) { - var isMatch = true; - foreach (var requiredLinkValue in entry.RequiredLinkValues) + var path = GenerateLink(context, entry); + if (path != null) { - if (!ContextHasSameValue(context, requiredLinkValue.Key, requiredLinkValue.Value)) - { - isMatch = false; - break; - } - } - - if (isMatch) - { - var path = GenerateLink(context, entry); - if (path != null) - { - context.IsBound = true; - return path; - } + context.IsBound = true; + return path; } } diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/Internal/Routing/LinkGenerationDecisionTreeTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/Internal/Routing/LinkGenerationDecisionTreeTest.cs new file mode 100644 index 0000000000..74510784a4 --- /dev/null +++ b/test/Microsoft.AspNet.Mvc.Core.Test/Internal/Routing/LinkGenerationDecisionTreeTest.cs @@ -0,0 +1,331 @@ +// 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 System.Collections.Generic; +using Microsoft.AspNet.Mvc.Routing; +using Microsoft.AspNet.PipelineCore; +using Microsoft.AspNet.Routing; +using Xunit; + +namespace Microsoft.AspNet.Mvc.Internal.Routing +{ + public class LinkGenerationDecisionTreeTest + { + [Fact] + public void SelectSingleEntry_NoCriteria() + { + // Arrange + var entries = new List(); + + var entry = CreateEntry(new { }); + entries.Add(entry); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext(new { }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Same(entry, Assert.Single(matches)); + } + + [Fact] + public void SelectSingleEntry_MultipleCriteria() + { + // Arrange + var entries = new List(); + + var entry = CreateEntry(new { controller = "Store", action = "Buy" }); + entries.Add(entry); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext(new { controller = "Store", action = "Buy" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Same(entry, Assert.Single(matches)); + } + + [Fact] + public void SelectSingleEntry_MultipleCriteria_AmbientValues() + { + // Arrange + var entries = new List(); + + var entry = CreateEntry(new { controller = "Store", action = "Buy" }); + entries.Add(entry); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext(values: null, ambientValues: new { controller = "Store", action = "Buy" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Same(entry, Assert.Single(matches)); + } + + [Fact] + public void SelectSingleEntry_MultipleCriteria_Replaced() + { + // Arrange + var entries = new List(); + + var entry = CreateEntry(new { controller = "Store", action = "Buy" }); + entries.Add(entry); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext( + values: new { action = "Buy" }, + ambientValues: new { controller = "Store", action = "Cart" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Same(entry, Assert.Single(matches)); + } + + [Fact] + public void SelectSingleEntry_MultipleCriteria_AmbientValue_Ignored() + { + // Arrange + var entries = new List(); + + var entry = CreateEntry(new { controller = "Store", action = (string)null }); + entries.Add(entry); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext( + values: new { controller = "Store" }, + ambientValues: new { controller = "Store", action = "Buy" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Same(entry, Assert.Single(matches)); + } + + [Fact] + public void SelectSingleEntry_MultipleCriteria_NoMatch() + { + // Arrange + var entries = new List(); + + var entry = CreateEntry(new { controller = "Store", action = "Buy" }); + entries.Add(entry); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext(new { controller = "Store", action = "AddToCart" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Empty(matches); + } + + [Fact] + public void SelectSingleEntry_MultipleCriteria_AmbientValue_NoMatch() + { + // Arrange + var entries = new List(); + + var entry = CreateEntry(new { controller = "Store", action = "Buy" }); + entries.Add(entry); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext( + values: new { controller = "Store" }, + ambientValues: new { controller = "Store", action = "Cart" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Empty(matches); + } + + [Fact] + public void SelectMultipleEntries_OneDoesntMatch() + { + // Arrange + var entries = new List(); + + var entry1 = CreateEntry(new { controller = "Store", action = "Buy" }); + entries.Add(entry1); + + var entry2 = CreateEntry(new { controller = "Store", action = "Cart" }); + entries.Add(entry2); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext( + values: new { controller = "Store" }, + ambientValues: new { controller = "Store", action = "Buy" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Same(entry1, Assert.Single(matches)); + } + + [Fact] + public void SelectMultipleEntries_BothMatch_CriteriaSubset() + { + // Arrange + var entries = new List(); + + var entry1 = CreateEntry(new { controller = "Store", action = "Buy" }); + entries.Add(entry1); + + var entry2 = CreateEntry(new { controller = "Store" }); + entry2.Order = 1; + entries.Add(entry2); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext( + values: new { controller = "Store" }, + ambientValues: new { controller = "Store", action = "Buy" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Equal(entries, matches); + } + + [Fact] + public void SelectMultipleEntries_BothMatch_NonOverlappingCriteria() + { + // Arrange + var entries = new List(); + + var entry1 = CreateEntry(new { controller = "Store", action = "Buy" }); + entries.Add(entry1); + + var entry2 = CreateEntry(new { slug = "1234" }); + entry2.Order = 1; + entries.Add(entry2); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext(new { controller = "Store", action = "Buy", slug = "1234" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Equal(entries, matches); + } + + // Precedence is ignored for sorting because they have different order + [Fact] + public void SelectMultipleEntries_BothMatch_OrderedByOrder() + { + // Arrange + var entries = new List(); + + var entry1 = CreateEntry(new { controller = "Store", action = "Buy" }); + entry1.Precedence = 1; + entries.Add(entry1); + + var entry2 = CreateEntry(new { controller = "Store", action = "Buy" }); + entry2.Order = 1; + entry2.Precedence = 0; + entries.Add(entry2); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext(new { controller = "Store", action = "Buy" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Equal(entries, matches); + } + + // Precedence is used for sorting because they have the same order + [Fact] + public void SelectMultipleEntries_BothMatch_OrderedByPrecedence() + { + // Arrange + var entries = new List(); + + var entry1 = CreateEntry(new { controller = "Store", action = "Buy" }); + entry1.Precedence = 0; + entries.Add(entry1); + + var entry2 = CreateEntry(new { controller = "Store", action = "Buy" }); + entry2.Precedence = 1; + entries.Add(entry2); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext(new { controller = "Store", action = "Buy" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Equal(entries, matches); + } + + // Template is used for sorting because they have the same order + [Fact] + public void SelectMultipleEntries_BothMatch_OrderedByTemplate() + { + // Arrange + var entries = new List(); + + var entry1 = CreateEntry(new { controller = "Store", action = "Buy" }); + entry1.TemplateText = "a"; + entries.Add(entry1); + + var entry2 = CreateEntry(new { controller = "Store", action = "Buy" }); + entry2.TemplateText = "b"; + entries.Add(entry2); + + var tree = new LinkGenerationDecisionTree(entries); + + var context = CreateContext(new { controller = "Store", action = "Buy" }); + + // Act + var matches = tree.GetMatches(context); + + // Assert + Assert.Equal(entries, matches); + } + + private AttributeRouteLinkGenerationEntry CreateEntry(object requiredValues) + { + var entry = new AttributeRouteLinkGenerationEntry(); + entry.RequiredLinkValues = new RouteValueDictionary(requiredValues); + return entry; + } + + private VirtualPathContext CreateContext(object values, object ambientValues = null) + { + var context = new VirtualPathContext( + new DefaultHttpContext(), + new RouteValueDictionary(ambientValues), + new RouteValueDictionary(values)); + + return context; + } + } +} \ No newline at end of file