From 6c5b836070182ba294b5f2032093b1dc4298a4c6 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Mon, 7 Jul 2014 19:43:50 -0700 Subject: [PATCH] Removing magic link generation --- .../DefaultActionSelector.cs | 193 ------------------ .../IActionSelector.cs | 2 - src/Microsoft.AspNet.Mvc.Core/UrlHelper.cs | 18 -- .../DefaultActionSelectorTest.cs | 149 -------------- 4 files changed, 362 deletions(-) diff --git a/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs b/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs index 2f52ff3949..956bb9d4ae 100644 --- a/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs +++ b/src/Microsoft.AspNet.Mvc.Core/DefaultActionSelector.cs @@ -164,180 +164,6 @@ namespace Microsoft.AspNet.Mvc return actions.Any(); } - // This is called by the default UrlHelper as part of Action link generation. When a link is requested - // specifically for an Action, we manipulate the route data to ensure that the right link is generated. - // Read further for details. - public virtual IEnumerable GetCandidateActions(VirtualPathContext context) - { - // This method attempts to find a unique 'best' candidate set of actions from the provided route - // values and ambient route values. - // - // The purpose of this process is to avoid allowing certain routes to be too greedy. When a route uses - // a default value as a filter, it can generate links to actions it will never hit. The actions returned - // by this method are used by the link generation code to manipulate the route values so that routes that - // are are greedy can't generate a link. - // - // The best example of this greediness is the canonical 'area' route from MVC. - // - // Ex: Areas/Admin/{controller}/{action} (defaults { area = "Admin" }) - // - // This route can generate a link even when the 'area' token is not provided. - // - // - // We define 'best' based on the combination of Values and AmbientValues. This set can be used to select a - // set of actions, anything in this is set is 'reachable'. We determine 'best' by looking for the - // 'reachable' actions ordered by the most total constraints matched, then the most constraints matched by - // ambient values. - // - // Ex: - // Consider the following actions - Home/Index (no area), and Admin/Home/Index (area = Admin). - // ambient values = { area = "Admin", controller = "Home", action = "Diagnostics" } - // values = { action = "Index" } - // - // In this case we want to select the Admin/Home/Index action, and algorithm leads us there. - // - // Admin/Home/Index: Total score 3, Explicit score 2, Implicit score 1, Omission score 0 - // Home/Index: Total score 3, Explicit score 2, Implicit score 0, Omission score 1 - // - // The description here is based on the concepts we're using to implement areas in WebFx, but apply - // to any tokens that might be used in routing (including REST conventions when action == null). - // - // This method does not take httpmethod or dynamic action constraints into account. - - var actions = GetActions(); - - var candidates = new List(); - foreach (var action in actions) - { - var candidate = new ActionDescriptorLinkCandidate() { Action = action }; - if (action.RouteConstraints == null) - { - candidates.Add(candidate); - continue; - } - - var isActionValid = true; - foreach (var constraint in action.RouteConstraints) - { - if (constraint.Accept(context.Values)) - { - if (context.Values.ContainsKey(constraint.RouteKey)) - { - // Explicit value is acceptable - candidate.ExplicitMatches++; - } - else - { - // No value supplied and that's OK for this action. - candidate.OmissionMatches++; - } - } - else if (context.Values.ContainsKey(constraint.RouteKey)) - { - // There's an explicitly provided value, but the action constraint doesn't match it. - isActionValid = false; - break; - } - else if (constraint.Accept(context.AmbientValues)) - { - // Ambient value is acceptable, used as a fallback - candidate.ImplicitMatches++; - } - else - { - // No possible match - isActionValid = false; - break; - } - } - - if (isActionValid) - { - candidates.Add(candidate); - } - } - - if (candidates.Count == 0) - { - return Enumerable.Empty(); - } - - // Finds all of the actions with the maximum number of total constraint matches. - var longestMatches = - candidates - .GroupBy(c => c.TotalMatches) - .OrderByDescending(g => g.Key) - .First(); - - // Finds all of the actions (from the above set) with the maximum number of explicit constraint matches. - var bestMatchesByExplicit = - longestMatches - .GroupBy(c => c.ExplicitMatches) - .OrderByDescending(g => g.Key) - .First(); - - // Finds all of the actions (from the above set) with the maximum number of implicit constraint matches. - var bestMatchesByImplicit = - bestMatchesByExplicit - .GroupBy(c => c.ImplicitMatches) - .OrderByDescending(g => g.Key) - .First(); - - var bestActions = bestMatchesByImplicit.Select(m => m.Action).ToArray(); - if (bestActions.Length == 1) - { - return bestActions; - } - - var exemplar = FindEquivalenceClass(bestActions); - if (exemplar == null) - { - throw new InvalidOperationException(Resources.ActionSelector_GetCandidateActionsIsAmbiguous); - } - else - { - return bestActions; - } - } - - // This method determines if the set of action descriptor candidates share a common set - // of route constraints, and returns an exemplar if there's a single set. This identifies - // a type of ambiguity, more data must be specified to ensure the right action can be selected. - // - // This is a no-op for our default conventions, but becomes important with custom action - // descriptor providers. - // - // Ex: These are not in the same equivalence class. - // Action 1: constraint keys - { action, controller, area } - // Action 2: constraint keys - { action, module } - private ActionDescriptor FindEquivalenceClass(ActionDescriptor[] candidates) - { - Contract.Assert(candidates.Length > 1); - - var criteria = new HashSet(StringComparer.OrdinalIgnoreCase); - - var exemplar = candidates[0]; - foreach (var constraint in exemplar.RouteConstraints) - { - criteria.Add(constraint.RouteKey); - } - - for (var i = 1; i < candidates.Length; i++) - { - var candidate = candidates[i]; - foreach (var constraint in exemplar.RouteConstraints) - { - if (criteria.Add(constraint.RouteKey)) - { - // This is a new criterion - the candidates have multiple criteria sets - return null; - } - } - } - - return exemplar; - } - private IReadOnlyList GetActions() { var descriptors = _actionDescriptorsCollectionProvider.ActionDescriptors; @@ -360,24 +186,5 @@ namespace Microsoft.AspNet.Mvc public int FoundOptionalParameters { get; set; } } - - private class ActionDescriptorLinkCandidate - { - public ActionDescriptor Action { get; set; } - - // Matches from explicit route values - public int ExplicitMatches { get; set; } - - // Matches from ambient route values - public int ImplicitMatches { get; set; } - - // Matches from explicit route values (by omission) - public int OmissionMatches { get; set; } - - public int TotalMatches - { - get { return ExplicitMatches + ImplicitMatches + OmissionMatches; } - } - } } } diff --git a/src/Microsoft.AspNet.Mvc.Core/IActionSelector.cs b/src/Microsoft.AspNet.Mvc.Core/IActionSelector.cs index 85c44a14d4..170dfbd050 100644 --- a/src/Microsoft.AspNet.Mvc.Core/IActionSelector.cs +++ b/src/Microsoft.AspNet.Mvc.Core/IActionSelector.cs @@ -14,7 +14,5 @@ namespace Microsoft.AspNet.Mvc bool Match(ActionDescriptor descriptor, RouteContext context); bool HasValidAction(VirtualPathContext context); - - IEnumerable GetCandidateActions(VirtualPathContext context); } } diff --git a/src/Microsoft.AspNet.Mvc.Core/UrlHelper.cs b/src/Microsoft.AspNet.Mvc.Core/UrlHelper.cs index 4c792811d2..5657e6e6c1 100644 --- a/src/Microsoft.AspNet.Mvc.Core/UrlHelper.cs +++ b/src/Microsoft.AspNet.Mvc.Core/UrlHelper.cs @@ -46,24 +46,6 @@ namespace Microsoft.AspNet.Mvc valuesDictionary["controller"] = controller; } - var context = new VirtualPathContext(_httpContext, _ambientValues, valuesDictionary); - var actions = _actionSelector.GetCandidateActions(context); - - var actionCandidate = actions.FirstOrDefault(); - if (actionCandidate == null) - { - return null; - } - - foreach (var constraint in actionCandidate.RouteConstraints) - { - if (constraint.KeyHandling == RouteKeyHandling.DenyKey && - _ambientValues.ContainsKey(constraint.RouteKey)) - { - valuesDictionary[constraint.RouteKey] = null; - } - } - var path = GeneratePathFromRoute(valuesDictionary); if (path == null) { diff --git a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTest.cs b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTest.cs index dd38df11ca..c314f19e9e 100644 --- a/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTest.cs +++ b/test/Microsoft.AspNet.Mvc.Core.Test/DefaultActionSelectorTest.cs @@ -7,7 +7,6 @@ using System.Linq; using System.Threading.Tasks; using Microsoft.AspNet.Http; using Microsoft.AspNet.Routing; -using Microsoft.Framework.DependencyInjection; using Moq; using Xunit; @@ -15,154 +14,6 @@ namespace Microsoft.AspNet.Mvc.Core.Test { public class DefaultActionSelectorTest { - [Fact] - public void GetCandidateActions_Match_NonArea() - { - // Arrange - var actions = GetActions(); - var expected = GetActions(actions, area: null, controller: "Home", action: "Index"); - - var selector = CreateSelector(actions); - var context = CreateContext(new { controller = "Home", action = "Index" }); - - // Act - var candidates = selector.GetCandidateActions(context); - - // Assert - Assert.Equal(expected, candidates); - } - - [Fact] - public void GetCandidateActions_Match_AreaExplicit() - { - // Arrange - var actions = GetActions(); - var expected = GetActions(actions, area: "Admin", controller: "Home", action: "Index"); - - var selector = CreateSelector(actions); - var context = CreateContext(new { area = "Admin", controller = "Home", action = "Index" }); - - // Act - var candidates = selector.GetCandidateActions(context); - - // Assert - Assert.Equal(expected, candidates); - } - - [Fact] - public void GetCandidateActions_Match_AreaImplicit() - { - // Arrange - var actions = GetActions(); - var expected = GetActions(actions, area: "Admin", controller: "Home", action: "Index"); - - var selector = CreateSelector(actions); - var context = CreateContext( - new { controller = "Home", action = "Index" }, - new { area = "Admin", controller = "Home", action = "Diagnostics" }); - - // Act - var candidates = selector.GetCandidateActions(context); - - // Assert - Assert.Equal(expected, candidates); - } - - [Fact] - public void GetCandidateActions_Match_NonAreaImplicit() - { - // Arrange - var actions = GetActions(); - var expected = GetActions(actions, area: null, controller: "Home", action: "Edit"); - - var selector = CreateSelector(actions); - var context = CreateContext( - new { controller = "Home", action = "Edit" }, - new { area = "Admin", controller = "Home", action = "Diagnostics" }); - - // Act - var candidates = selector.GetCandidateActions(context); - - // Assert - Assert.Equal(expected, candidates); - } - - [Fact] - public void GetCandidateActions_Match_NonAreaExplicit() - { - // Arrange - var actions = GetActions(); - var expected = GetActions(actions, area: null, controller: "Home", action: "Index"); - - var selector = CreateSelector(actions); - var context = CreateContext( - new { area = (string)null, controller = "Home", action = "Index" }, - new { area = "Admin", controller = "Home", action = "Diagnostics" }); - - // Act - var candidates = selector.GetCandidateActions(context); - - // Assert - Assert.Equal(expected, candidates); - } - - [Fact] - public void GetCandidateActions_Match_RestExplicit() - { - // Arrange - var actions = GetActions(); - var expected = GetActions(actions, area: null, controller: "Product", action: null); - - var selector = CreateSelector(actions); - var context = CreateContext( - new { controller = "Product", action = (string)null }, - new { controller = "Home", action = "Index" }); - - // Act - var candidates = selector.GetCandidateActions(context); - - // Assert - Assert.Equal(expected, candidates); - } - - [Fact] - public void GetCandidateActions_Match_RestImplicit() - { - // Arrange - var actions = GetActions(); - var expected = GetActions(actions, area: null, controller: "Product", action: null); - - var selector = CreateSelector(actions); - var context = CreateContext( - new { controller = "Product" }, - new { controller = "Home", action = "Index" }); - - // Act - var candidates = selector.GetCandidateActions(context); - - // Assert - Assert.Equal(expected, candidates); - } - - - [Fact] - public void GetCandidateActions_NoMatch() - { - // Arrange - var actions = GetActions(); - - var selector = CreateSelector(actions); - var context = CreateContext( - new { area = "Admin", controller = "Home", action = "Edit" }, - new { area = "Admin", controller = "Home", action = "Index" }); - - // Act - var candidates = selector.GetCandidateActions(context); - - // Assert - Assert.Empty(candidates); - } - [Fact] public void HasValidAction_Match() {