From f8b3b73ca7cd31443bf64cf70d0497a91d2078b4 Mon Sep 17 00:00:00 2001 From: Ryan Nowak Date: Thu, 26 Jul 2018 23:20:44 -0700 Subject: [PATCH] Remove EndpointConstraints Adds IEndpointSelectorPolicy so that MVC can plug in to the EndpointSelector to run action constraints. --- .../Matchers/MatcherBenchmarkBase.cs | 1 - .../CompositeEndpointDataSource.cs | 8 +- .../RoutingServiceCollectionExtensions.cs | 10 +- .../EndpointConstraintCache.cs | 208 ------- .../EndpointConstraintEndpointSelector.cs | 265 --------- .../HttpMethodEndpointConstraint.cs | 75 --- .../IEndpointConstraint.cs | 190 ------- .../Matchers/DefaultEndpointSelector.cs | 25 +- .../Matchers/IEndpointSelectorPolicy.cs | 12 + .../EndpointConstraintEndpointSelectorTest.cs | 511 ------------------ .../Matchers/DefaultEndpointSelectorTest.cs | 36 +- .../Matchers/RouteMatcherBuilder.cs | 7 +- .../Matchers/TreeRouterMatcherBuilder.cs | 6 +- 13 files changed, 73 insertions(+), 1281 deletions(-) delete mode 100644 src/Microsoft.AspNetCore.Routing/EndpointConstraints/EndpointConstraintCache.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/EndpointConstraints/EndpointConstraintEndpointSelector.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/EndpointConstraints/HttpMethodEndpointConstraint.cs delete mode 100644 src/Microsoft.AspNetCore.Routing/EndpointConstraints/IEndpointConstraint.cs create mode 100644 src/Microsoft.AspNetCore.Routing/Matchers/IEndpointSelectorPolicy.cs delete mode 100644 test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointConstraintEndpointSelectorTest.cs diff --git a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs index 612e4db9f3..8d428e8253 100644 --- a/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs +++ b/benchmarks/Microsoft.AspNetCore.Routing.Performance/Matchers/MatcherBenchmarkBase.cs @@ -7,7 +7,6 @@ using System.Runtime.CompilerServices; using System.Text; using System.Threading.Tasks; using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Routing.EndpointConstraints; using Microsoft.AspNetCore.Routing.Metadata; using Microsoft.AspNetCore.Routing.Patterns; using Microsoft.Extensions.DependencyInjection; diff --git a/src/Microsoft.AspNetCore.Routing/CompositeEndpointDataSource.cs b/src/Microsoft.AspNetCore.Routing/CompositeEndpointDataSource.cs index 27629fd1a3..2b4912a086 100644 --- a/src/Microsoft.AspNetCore.Routing/CompositeEndpointDataSource.cs +++ b/src/Microsoft.AspNetCore.Routing/CompositeEndpointDataSource.cs @@ -7,7 +7,6 @@ using System.Diagnostics; using System.Linq; using System.Text; using System.Threading; -using Microsoft.AspNetCore.Routing.EndpointConstraints; using Microsoft.AspNetCore.Routing.Matchers; using Microsoft.AspNetCore.Routing.Metadata; using Microsoft.Extensions.Primitives; @@ -138,11 +137,8 @@ namespace Microsoft.AspNetCore.Routing var httpMethodMetadata = matcherEndpoint.Metadata.GetMetadata(); if (httpMethodMetadata != null) { - foreach (var httpMethod in httpMethodMetadata.HttpMethods) - { - sb.Append(", Http Methods: "); - sb.Append(string.Join(", ", httpMethod)); - } + sb.Append(", Http Methods: "); + sb.Append(string.Join(", ", httpMethodMetadata.HttpMethods)); } sb.AppendLine(); diff --git a/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs b/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs index 0e335d7d46..ab4c29b702 100644 --- a/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs +++ b/src/Microsoft.AspNetCore.Routing/DependencyInjection/RoutingServiceCollectionExtensions.cs @@ -4,7 +4,6 @@ using System; using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.Routing.Constraints; -using Microsoft.AspNetCore.Routing.EndpointConstraints; using Microsoft.AspNetCore.Routing.EndpointFinders; using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Matchers; @@ -73,18 +72,13 @@ namespace Microsoft.Extensions.DependencyInjection services.TryAddSingleton, NameBasedEndpointFinder>(); services.TryAddSingleton, RouteValuesBasedEndpointFinder>(); services.TryAddSingleton(); - + // // Endpoint Selection // - services.TryAddSingleton(); - services.TryAddSingleton(); + services.TryAddSingleton(); services.TryAddEnumerable(ServiceDescriptor.Singleton()); - // Will be cached by the EndpointSelector - services.TryAddEnumerable( - ServiceDescriptor.Transient()); - return services; } diff --git a/src/Microsoft.AspNetCore.Routing/EndpointConstraints/EndpointConstraintCache.cs b/src/Microsoft.AspNetCore.Routing/EndpointConstraints/EndpointConstraintCache.cs deleted file mode 100644 index 1a5c4a350c..0000000000 --- a/src/Microsoft.AspNetCore.Routing/EndpointConstraints/EndpointConstraintCache.cs +++ /dev/null @@ -1,208 +0,0 @@ -// Copyright (c) .NET Foundation. All rights reserved. -// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. - -using Microsoft.AspNetCore.Http; -using System; -using System.Collections.Concurrent; -using System.Collections.Generic; -using System.Diagnostics; -using System.Linq; - -namespace Microsoft.AspNetCore.Routing.EndpointConstraints -{ - internal class EndpointConstraintCache - { - private readonly CompositeEndpointDataSource _dataSource; - private readonly IEndpointConstraintProvider[] _endpointConstraintProviders; - - private volatile InnerCache _currentCache; - - public EndpointConstraintCache( - CompositeEndpointDataSource dataSource, - IEnumerable endpointConstraintProviders) - { - _dataSource = dataSource; - _endpointConstraintProviders = endpointConstraintProviders.OrderBy(item => item.Order).ToArray(); - } - - private InnerCache CurrentCache - { - get - { - var current = _currentCache; - var endpointDescriptors = _dataSource.Endpoints; - - if (current == null) - { - current = new InnerCache(); - _currentCache = current; - } - - return current; - } - } - - public IReadOnlyList GetEndpointConstraints(HttpContext httpContext, Endpoint endpoint) - { - var cache = CurrentCache; - - if (cache.Entries.TryGetValue(endpoint, out var entry)) - { - return GetEndpointConstraintsFromEntry(entry, httpContext, endpoint); - } - - List items = null; - - if (endpoint.Metadata != null && endpoint.Metadata.Count > 0) - { - items = endpoint.Metadata - .OfType() - .Select(m => new EndpointConstraintItem(m)) - .ToList(); - } - - IReadOnlyList endpointConstraints = null; - - if (items != null && items.Count > 0) - { - ExecuteProviders(httpContext, endpoint, items); - - endpointConstraints = ExtractEndpointConstraints(items); - - var allEndpointConstraintsCached = true; - for (var i = 0; i < items.Count; i++) - { - var item = items[i]; - if (!item.IsReusable) - { - item.Constraint = null; - allEndpointConstraintsCached = false; - } - } - - if (allEndpointConstraintsCached) - { - entry = new CacheEntry(endpointConstraints); - } - else - { - entry = new CacheEntry(items); - } - } - else - { - // No constraints - entry = new CacheEntry(); - } - - cache.Entries.TryAdd(endpoint, entry); - return endpointConstraints; - } - - private IReadOnlyList GetEndpointConstraintsFromEntry(CacheEntry entry, HttpContext httpContext, Endpoint endpoint) - { - if (entry.EndpointConstraints != null) - { - return entry.EndpointConstraints; - } - - if (entry.Items == null) - { - // Endpoint has no constraints - return null; - } - - var items = new List(entry.Items.Count); - for (var i = 0; i < entry.Items.Count; i++) - { - var item = entry.Items[i]; - if (item.IsReusable) - { - items.Add(item); - } - else - { - items.Add(new EndpointConstraintItem(item.Metadata)); - } - } - - ExecuteProviders(httpContext, endpoint, items); - - return ExtractEndpointConstraints(items); - } - - private void ExecuteProviders(HttpContext httpContext, Endpoint endpoint, List items) - { - var context = new EndpointConstraintProviderContext(httpContext, endpoint, items); - - for (var i = 0; i < _endpointConstraintProviders.Length; i++) - { - _endpointConstraintProviders[i].OnProvidersExecuting(context); - } - - for (var i = _endpointConstraintProviders.Length - 1; i >= 0; i--) - { - _endpointConstraintProviders[i].OnProvidersExecuted(context); - } - } - - private IReadOnlyList ExtractEndpointConstraints(List items) - { - var count = 0; - for (var i = 0; i < items.Count; i++) - { - if (items[i].Constraint != null) - { - count++; - } - } - - if (count == 0) - { - return null; - } - - var endpointConstraints = new IEndpointConstraint[count]; - var endpointConstraintIndex = 0; - for (int i = 0; i < items.Count; i++) - { - var endpointConstraint = items[i].Constraint; - if (endpointConstraint != null) - { - endpointConstraints[endpointConstraintIndex++] = endpointConstraint; - } - } - - return endpointConstraints; - } - - private class InnerCache - { - public InnerCache() - { - } - - public ConcurrentDictionary Entries { get; } = - new ConcurrentDictionary(); - } - - private struct CacheEntry - { - public CacheEntry(IReadOnlyList endpointConstraints) - { - EndpointConstraints = endpointConstraints; - Items = null; - } - - public CacheEntry(List items) - { - Items = items; - EndpointConstraints = null; - } - - public IReadOnlyList EndpointConstraints { get; } - - public List Items { get; } - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Routing/EndpointConstraints/EndpointConstraintEndpointSelector.cs b/src/Microsoft.AspNetCore.Routing/EndpointConstraints/EndpointConstraintEndpointSelector.cs deleted file mode 100644 index f063079f87..0000000000 --- a/src/Microsoft.AspNetCore.Routing/EndpointConstraints/EndpointConstraintEndpointSelector.cs +++ /dev/null @@ -1,265 +0,0 @@ -// Copyright (c) .NET Foundation. 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 System.Linq; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Routing.Matchers; -using Microsoft.Extensions.Logging; - -namespace Microsoft.AspNetCore.Routing.EndpointConstraints -{ - internal class EndpointConstraintEndpointSelector : EndpointSelector - { - private static readonly IReadOnlyList EmptyEndpoints = Array.Empty(); - - private readonly CompositeEndpointDataSource _dataSource; - private readonly EndpointConstraintCache _endpointConstraintCache; - private readonly ILogger _logger; - - public EndpointConstraintEndpointSelector( - CompositeEndpointDataSource dataSource, - EndpointConstraintCache endpointConstraintCache, - ILoggerFactory loggerFactory) - { - _dataSource = dataSource; - _logger = loggerFactory.CreateLogger(); - _endpointConstraintCache = endpointConstraintCache; - } - - public override Task SelectAsync( - HttpContext httpContext, - IEndpointFeature feature, - CandidateSet candidates) - { - if (httpContext == null) - { - throw new ArgumentNullException(nameof(httpContext)); - } - - if (feature == null) - { - throw new ArgumentNullException(nameof(feature)); - } - - if (candidates == null) - { - throw new ArgumentNullException(nameof(candidates)); - } - - var finalMatches = EvaluateEndpointConstraints(httpContext, candidates); - - if (finalMatches == null || finalMatches.Count == 0) - { - return Task.CompletedTask; - } - else if (finalMatches.Count == 1) - { - var endpoint = finalMatches[0].Endpoint; - var values = finalMatches[0].Values; - - feature.Endpoint = endpoint; - feature.Invoker = (endpoint as MatcherEndpoint)?.Invoker; - feature.Values = values; - - return Task.CompletedTask; - } - else - { - var endpointNames = string.Join( - Environment.NewLine, - finalMatches.Select(a => a.Endpoint.DisplayName)); - - Log.MatchAmbiguous(_logger, httpContext, finalMatches); - - var message = Resources.FormatAmbiguousEndpoints( - Environment.NewLine, - string.Join(Environment.NewLine, endpointNames)); - - throw new AmbiguousMatchException(message); - } - } - - private IReadOnlyList EvaluateEndpointConstraints( - HttpContext context, - CandidateSet candidateSet) - { - var candidates = new List(); - - // Perf: Avoid allocations - for (var i = 0; i < candidateSet.Count; i++) - { - ref var candidate = ref candidateSet[i]; - if (candidate.IsValidCandidate) - { - var endpoint = candidate.Endpoint; - var constraints = _endpointConstraintCache.GetEndpointConstraints(context, endpoint); - candidates.Add(new EndpointSelectorCandidate( - endpoint, - candidate.Score, - candidate.Values, - constraints)); - } - } - - var matches = EvaluateEndpointConstraintsCore(context, candidates, startingOrder: null); - - List results = null; - if (matches != null) - { - results = new List(matches.Count); - - // We need to disambiguate based on 'score' - take the first value of 'score' - // and then we only copy matches while they have the same score. This accounts - // for a difference in behavior between new routing and old. - switch (matches.Count) - { - case 0: - break; - - case 1: - results.Add(matches[0]); - break; - - default: - var score = matches[0].Score; - for (var i = 0; i < matches.Count; i++) - { - if (matches[i].Score != score) - { - break; - } - - results.Add(matches[i]); - } - - break; - - } - } - - return results; - } - - private IReadOnlyList EvaluateEndpointConstraintsCore( - HttpContext context, - IReadOnlyList candidates, - int? startingOrder) - { - // Find the next group of constraints to process. This will be the lowest value of - // order that is higher than startingOrder. - int? order = null; - - // Perf: Avoid allocations - for (var i = 0; i < candidates.Count; i++) - { - var candidate = candidates[i]; - if (candidate.Constraints != null) - { - for (var j = 0; j < candidate.Constraints.Count; j++) - { - var constraint = candidate.Constraints[j]; - if ((startingOrder == null || constraint.Order > startingOrder) && - (order == null || constraint.Order < order)) - { - order = constraint.Order; - } - } - } - } - - // If we don't find a next then there's nothing left to do. - if (order == null) - { - return candidates; - } - - // Since we have a constraint to process, bisect the set of endpoints into those with and without a - // constraint for the current order. - var endpointsWithConstraint = new List(); - var endpointsWithoutConstraint = new List(); - - var constraintContext = new EndpointConstraintContext(); - constraintContext.Candidates = candidates; - constraintContext.HttpContext = context; - - // Perf: Avoid allocations - for (var i = 0; i < candidates.Count; i++) - { - var candidate = candidates[i]; - var isMatch = true; - var foundMatchingConstraint = false; - - if (candidate.Constraints != null) - { - constraintContext.CurrentCandidate = candidate; - for (var j = 0; j < candidate.Constraints.Count; j++) - { - var constraint = candidate.Constraints[j]; - if (constraint.Order == order) - { - foundMatchingConstraint = true; - - if (!constraint.Accept(constraintContext)) - { - isMatch = false; - //_logger.ConstraintMismatch( - // candidate.Endpoint.DisplayName, - // candidate.Endpoint.Id, - // constraint); - break; - } - } - } - } - - if (isMatch && foundMatchingConstraint) - { - endpointsWithConstraint.Add(candidate); - } - else if (isMatch) - { - endpointsWithoutConstraint.Add(candidate); - } - } - - // If we have matches with constraints, those are better so try to keep processing those - if (endpointsWithConstraint.Count > 0) - { - var matches = EvaluateEndpointConstraintsCore(context, endpointsWithConstraint, order); - if (matches?.Count > 0) - { - return matches; - } - } - - // If the set of matches with constraints can't work, then process the set without constraints. - if (endpointsWithoutConstraint.Count == 0) - { - return null; - } - else - { - return EvaluateEndpointConstraintsCore(context, endpointsWithoutConstraint, order); - } - } - - private static class Log - { - private static readonly Action, Exception> _matchAmbiguous = LoggerMessage.Define>( - LogLevel.Error, - new EventId(1, "MatchAmbiguous"), - "Request matched multiple endpoints for request path '{Path}'. Matching endpoints: {AmbiguousEndpoints}"); - - public static void MatchAmbiguous(ILogger logger, HttpContext httpContext, IEnumerable endpoints) - { - if (logger.IsEnabled(LogLevel.Error)) - { - _matchAmbiguous(logger, httpContext.Request.Path, endpoints.Select(e => e.Endpoint.DisplayName), null); - } - } - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Routing/EndpointConstraints/HttpMethodEndpointConstraint.cs b/src/Microsoft.AspNetCore.Routing/EndpointConstraints/HttpMethodEndpointConstraint.cs deleted file mode 100644 index 296986d523..0000000000 --- a/src/Microsoft.AspNetCore.Routing/EndpointConstraints/HttpMethodEndpointConstraint.cs +++ /dev/null @@ -1,75 +0,0 @@ -// Copyright (c) .NET Foundation. 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 System.Collections.ObjectModel; -using Microsoft.AspNetCore.Routing.Metadata; - -namespace Microsoft.AspNetCore.Routing.EndpointConstraints -{ - public class HttpMethodEndpointConstraint : IEndpointConstraint, IHttpMethodMetadata - { - public static readonly int HttpMethodConstraintOrder = 100; - - private readonly IReadOnlyList _httpMethods; - - // Empty collection means any method will be accepted. - public HttpMethodEndpointConstraint(IEnumerable httpMethods) - { - if (httpMethods == null) - { - throw new ArgumentNullException(nameof(httpMethods)); - } - - var methods = new List(); - - foreach (var method in httpMethods) - { - if (string.IsNullOrEmpty(method)) - { - throw new ArgumentException("httpMethod cannot be null or empty"); - } - - methods.Add(method); - } - - _httpMethods = new ReadOnlyCollection(methods); - } - - public IEnumerable HttpMethods => _httpMethods; - - public int Order => HttpMethodConstraintOrder; - - IReadOnlyList IHttpMethodMetadata.HttpMethods => _httpMethods; - - bool IHttpMethodMetadata.AcceptCorsPreflight => false; - - public virtual bool Accept(EndpointConstraintContext context) - { - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } - - if (_httpMethods.Count == 0) - { - return true; - } - - var request = context.HttpContext.Request; - var method = request.Method; - - for (var i = 0; i < _httpMethods.Count; i++) - { - var supportedMethod = _httpMethods[i]; - if (string.Equals(supportedMethod, method, StringComparison.OrdinalIgnoreCase)) - { - return true; - } - } - - return false; - } - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Routing/EndpointConstraints/IEndpointConstraint.cs b/src/Microsoft.AspNetCore.Routing/EndpointConstraints/IEndpointConstraint.cs deleted file mode 100644 index 1a71ec738d..0000000000 --- a/src/Microsoft.AspNetCore.Routing/EndpointConstraints/IEndpointConstraint.cs +++ /dev/null @@ -1,190 +0,0 @@ -// Copyright (c) .NET Foundation. 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.AspNetCore.Http; - -namespace Microsoft.AspNetCore.Routing.EndpointConstraints -{ - public class EndpointConstraintContext - { - public IReadOnlyList Candidates { get; set; } - - public EndpointSelectorCandidate CurrentCandidate { get; set; } - - public HttpContext HttpContext { get; set; } - } - - public interface IEndpointConstraint : IEndpointConstraintMetadata - { - int Order { get; } - - bool Accept(EndpointConstraintContext context); - } - - public interface IEndpointConstraintMetadata - { - } - - public readonly struct EndpointSelectorCandidate - { - public EndpointSelectorCandidate( - Endpoint endpoint, - int score, - RouteValueDictionary values, - IReadOnlyList constraints) - { - if (endpoint == null) - { - throw new ArgumentNullException(nameof(endpoint)); - } - - Endpoint = endpoint; - Score = score; - Values = values; - Constraints = constraints; - } - - // Temporarily added to not break MVC build - public EndpointSelectorCandidate( - Endpoint endpoint, - IReadOnlyList constraints) - { - if (endpoint == null) - { - throw new ArgumentNullException(nameof(endpoint)); - } - - Endpoint = endpoint; - Score = 0; - Values = null; - Constraints = constraints; - } - - public Endpoint Endpoint { get; } - - public int Score { get; } - - public RouteValueDictionary Values { get; } - - public IReadOnlyList Constraints { get; } - } - - public class EndpointConstraintItem - { - public EndpointConstraintItem(IEndpointConstraintMetadata metadata) - { - if (metadata == null) - { - throw new ArgumentNullException(nameof(metadata)); - } - - Metadata = metadata; - } - - public IEndpointConstraint Constraint { get; set; } - - public IEndpointConstraintMetadata Metadata { get; } - - public bool IsReusable { get; set; } - } - - public interface IEndpointConstraintProvider - { - int Order { get; } - - void OnProvidersExecuting(EndpointConstraintProviderContext context); - - void OnProvidersExecuted(EndpointConstraintProviderContext context); - } - - public class EndpointConstraintProviderContext - { - public EndpointConstraintProviderContext( - HttpContext context, - Endpoint endpoint, - IList items) - { - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } - - if (endpoint == null) - { - throw new ArgumentNullException(nameof(endpoint)); - } - - if (items == null) - { - throw new ArgumentNullException(nameof(items)); - } - - HttpContext = context; - Endpoint = endpoint; - Results = items; - } - - public HttpContext HttpContext { get; } - - public Endpoint Endpoint { get; } - - public IList Results { get; } - } - - public class DefaultEndpointConstraintProvider : IEndpointConstraintProvider - { - /// - public int Order => -1000; - - /// - public void OnProvidersExecuting(EndpointConstraintProviderContext context) - { - if (context == null) - { - throw new ArgumentNullException(nameof(context)); - } - - for (var i = 0; i < context.Results.Count; i++) - { - ProvideConstraint(context.Results[i], context.HttpContext.RequestServices); - } - } - - /// - public void OnProvidersExecuted(EndpointConstraintProviderContext context) - { - } - - private void ProvideConstraint(EndpointConstraintItem item, IServiceProvider services) - { - // Don't overwrite anything that was done by a previous provider. - if (item.Constraint != null) - { - return; - } - - if (item.Metadata is IEndpointConstraint constraint) - { - item.Constraint = constraint; - item.IsReusable = true; - return; - } - - if (item.Metadata is IEndpointConstraintFactory factory) - { - item.Constraint = factory.CreateInstance(services); - item.IsReusable = factory.IsReusable; - return; - } - } - } - - public interface IEndpointConstraintFactory : IEndpointConstraintMetadata - { - bool IsReusable { get; } - - IEndpointConstraint CreateInstance(IServiceProvider services); - } -} \ No newline at end of file diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/DefaultEndpointSelector.cs b/src/Microsoft.AspNetCore.Routing/Matchers/DefaultEndpointSelector.cs index b76010851f..2d3e6d5026 100644 --- a/src/Microsoft.AspNetCore.Routing/Matchers/DefaultEndpointSelector.cs +++ b/src/Microsoft.AspNetCore.Routing/Matchers/DefaultEndpointSelector.cs @@ -11,17 +11,34 @@ namespace Microsoft.AspNetCore.Routing.Matchers { internal class DefaultEndpointSelector : EndpointSelector { + private readonly IEndpointSelectorPolicy[] _selectorPolicies; + + public DefaultEndpointSelector(IEnumerable matcherPolicies) + { + if (matcherPolicies == null) + { + throw new ArgumentNullException(nameof(matcherPolicies)); + } + + _selectorPolicies = matcherPolicies.OrderBy(p => p.Order).OfType().ToArray(); + } + public override Task SelectAsync( HttpContext httpContext, IEndpointFeature feature, - CandidateSet candidates) + CandidateSet candidateSet) { + for (var i = 0; i < _selectorPolicies.Length; i++) + { + _selectorPolicies[i].Apply(httpContext, candidateSet); + } + MatcherEndpoint endpoint = null; RouteValueDictionary values = null; int? foundScore = null; - for (var i = 0; i < candidates.Count; i++) + for (var i = 0; i < candidateSet.Count; i++) { - ref var state = ref candidates[i]; + ref var state = ref candidateSet[i]; var isValid = state.IsValidCandidate; if (isValid && foundScore == null) @@ -46,7 +63,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers // // Don't worry about the 'null == state.Score' case, it returns false. - ReportAmbiguity(candidates); + ReportAmbiguity(candidateSet); // Unreachable, ReportAmbiguity always throws. throw new NotSupportedException(); diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/IEndpointSelectorPolicy.cs b/src/Microsoft.AspNetCore.Routing/Matchers/IEndpointSelectorPolicy.cs new file mode 100644 index 0000000000..cc6fe09d9c --- /dev/null +++ b/src/Microsoft.AspNetCore.Routing/Matchers/IEndpointSelectorPolicy.cs @@ -0,0 +1,12 @@ +// Copyright (c) .NET Foundation. All rights reserved. +// Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. + +using Microsoft.AspNetCore.Http; + +namespace Microsoft.AspNetCore.Routing.Matchers +{ + public interface IEndpointSelectorPolicy + { + void Apply(HttpContext httpContext, CandidateSet candidates); + } +} diff --git a/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointConstraintEndpointSelectorTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointConstraintEndpointSelectorTest.cs deleted file mode 100644 index 72bf124785..0000000000 --- a/test/Microsoft.AspNetCore.Routing.Tests/EndpointConstraints/EndpointConstraintEndpointSelectorTest.cs +++ /dev/null @@ -1,511 +0,0 @@ -// Copyright (c) .NET Foundation. 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 System.Collections.ObjectModel; -using System.Linq; -using System.Threading.Tasks; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Routing.Matchers; -using Microsoft.AspNetCore.Routing.Patterns; -using Microsoft.Extensions.DependencyInjection; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; -using Microsoft.Extensions.Logging.Testing; -using Moq; -using Xunit; - -namespace Microsoft.AspNetCore.Routing.EndpointConstraints -{ - public class EndpointConstraintEndpointSelectorTest - { - [Fact] - public async Task SelectBestCandidate_MultipleEndpoints_BestMatchSelected() - { - // Arrange - var defaultEndpoint = CreateEndpoint("No constraint endpoint"); - - var postEndpoint = CreateEndpoint( - "POST constraint endpoint", - new HttpMethodEndpointConstraint(new[] { "POST" })); - - var endpoints = new[] - { - defaultEndpoint, - postEndpoint - }; - - var selector = CreateSelector(endpoints); - - var httpContext = new DefaultHttpContext(); - httpContext.Request.Method = "POST"; - - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - - // Assert - Assert.Same(postEndpoint, feature.Endpoint); - } - - [Fact] - public async Task SelectBestCandidate_MultipleEndpoints_AmbiguousMatchExceptionThrown() - { - // Arrange - var expectedMessage = - "The request matched multiple endpoints. Matches: " + Environment.NewLine + - Environment.NewLine + - "Ambiguous1" + Environment.NewLine + - "Ambiguous2"; - - var defaultEndpoint1 = CreateEndpoint("Ambiguous1"); - var defaultEndpoint2 = CreateEndpoint("Ambiguous2"); - - var endpoints = new[] - { - defaultEndpoint1, - defaultEndpoint2 - }; - - var selector = CreateSelector(endpoints); - - var httpContext = new DefaultHttpContext(); - httpContext.Request.Method = "POST"; - - var feature = new EndpointFeature(); - - // Act - var ex = await Assert.ThrowsAnyAsync(() => - { - return selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - }); - - // Assert - Assert.Equal(expectedMessage, ex.Message); - } - - [Fact] - public async Task SelectBestCandidate_AmbiguousEndpoints_LogIsCorrect() - { - // Arrange - var sink = new TestSink(); - var loggerFactory = new TestLoggerFactory(sink, enabled: true); - - var endpoints = new[] - { - CreateEndpoint("A1"), - CreateEndpoint("A2"), - }; - - var selector = CreateSelector(endpoints, loggerFactory); - - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - var names = string.Join(", ", endpoints.Select(action => action.DisplayName)); - var expectedMessage = - $"Request matched multiple endpoints for request path '/test'. " + - $"Matching endpoints: {names}"; - - // Act - await Assert.ThrowsAsync(() => - { - return selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - }); - - // Assert - Assert.Empty(sink.Scopes); - var write = Assert.Single(sink.Writes); - Assert.Equal(expectedMessage, write.State?.ToString()); - } - - [Fact] - public async Task SelectBestCandidate_PrefersEndpointWithConstraints() - { - // Arrange - var endpointWithConstraint = CreateEndpoint( - "Has constraint", - new HttpMethodEndpointConstraint(new string[] { "POST" })); - - var endpointWithoutConstraints = CreateEndpoint("No constraint"); - - var endpoints = new[] { endpointWithConstraint, endpointWithoutConstraints }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - - // Assert - Assert.Same(endpointWithConstraint, endpointWithConstraint); - } - - [Fact] - public async Task SelectBestCandidate_ConstraintsRejectAll() - { - // Arrange - var endpoint1 = CreateEndpoint( - "action1", - new BooleanConstraint() { Pass = false, }); - - var endpoint2 = CreateEndpoint( - "action2", - new BooleanConstraint() { Pass = false, }); - - var endpoints = new[] { endpoint1, endpoint2 }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - - // Assert - Assert.Null(feature.Endpoint); - } - - [Fact] - public async Task SelectBestCandidate_ConstraintsRejectAll_DifferentStages() - { - // Arrange - var endpoint1 = CreateEndpoint( - "action1", - new BooleanConstraint() { Pass = false, Order = 0 }, - new BooleanConstraint() { Pass = true, Order = 1 }); - - var endpoint2 = CreateEndpoint( - "action2", - new BooleanConstraint() { Pass = true, Order = 0 }, - new BooleanConstraint() { Pass = false, Order = 1 }); - - var endpoints = new[] { endpoint1, endpoint2 }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - - // Assert - Assert.Null(feature.Endpoint); - } - - [Fact] - public async Task SelectBestCandidate_EndpointConstraintFactory() - { - // Arrange - var endpointWithConstraints = CreateEndpoint( - "actionWithConstraints", - new ConstraintFactory() - { - Constraint = new BooleanConstraint() { Pass = true }, - }); - - var actionWithoutConstraints = CreateEndpoint("actionWithoutConstraints"); - - var endpoints = new[] { endpointWithConstraints, actionWithoutConstraints }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - - // Assert - Assert.Same(endpointWithConstraints, feature.Endpoint); - } - - [Fact] - public async Task SelectBestCandidate_MultipleCallsNoConstraint_ReturnsEndpoint() - { - // Arrange - var noConstraint = CreateEndpoint("noConstraint"); - - var endpoints = new[] { noConstraint }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - var endpoint1 = feature.Endpoint; - - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - var endpoint2 = feature.Endpoint; - - // Assert - Assert.Same(endpoint1, noConstraint); - Assert.Same(endpoint2, noConstraint); - } - - [Fact] - public async Task SelectBestCandidate_MultipleCallsNonConstraintMetadata_ReturnsEndpoint() - { - // Arrange - var noConstraint = CreateEndpoint("noConstraint", new object()); - - var endpoints = new[] { noConstraint }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - var endpoint1 = feature.Endpoint; - - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - var endpoint2 = feature.Endpoint; - - // Assert - Assert.Same(endpoint1, noConstraint); - Assert.Same(endpoint2, noConstraint); - } - - [Fact] - public async Task SelectBestCandidate_EndpointConstraintFactory_ReturnsNull() - { - // Arrange - var nullConstraint = CreateEndpoint("nullConstraint", new ConstraintFactory()); - - var endpoints = new[] { nullConstraint }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - var endpoint1 = feature.Endpoint; - - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - var endpoint2 = feature.Endpoint; - - // Assert - Assert.Same(endpoint1, nullConstraint); - Assert.Same(endpoint2, nullConstraint); - } - - // There's a custom constraint provider registered that only understands BooleanConstraintMarker - [Fact] - public async Task SelectBestCandidate_CustomProvider() - { - // Arrange - var endpointWithConstraints = CreateEndpoint( - "actionWithConstraints", - new BooleanConstraintMarker() { Pass = true }); - - var endpointWithoutConstraints = CreateEndpoint("actionWithoutConstraints"); - - var endpoints = new[] { endpointWithConstraints, endpointWithoutConstraints, }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - - // Assert - Assert.Same(endpointWithConstraints, feature.Endpoint); - } - - // Due to ordering of stages, the first action will be better. - [Fact] - public async Task SelectBestCandidate_ConstraintsInOrder() - { - // Arrange - var best = CreateEndpoint("best", new BooleanConstraint() { Pass = true, Order = 0, }); - - var worst = CreateEndpoint("worst", new BooleanConstraint() { Pass = true, Order = 1, }); - - var endpoints = new[] { best, worst }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - - // Assert - Assert.Same(best, feature.Endpoint); - } - - // Due to ordering of stages, the first action will be better. - [Fact] - public async Task SelectBestCandidate_ConstraintsInOrder_MultipleStages() - { - // Arrange - var best = CreateEndpoint( - "best", - new BooleanConstraint() { Pass = true, Order = 0, }, - new BooleanConstraint() { Pass = true, Order = 1, }, - new BooleanConstraint() { Pass = true, Order = 2, }); - - var worst = CreateEndpoint( - "worst", - new BooleanConstraint() { Pass = true, Order = 0, }, - new BooleanConstraint() { Pass = true, Order = 1, }, - new BooleanConstraint() { Pass = true, Order = 3, }); - - var endpoints = new[] { best, worst }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - - // Assert - Assert.Same(best, feature.Endpoint); - } - - [Fact] - public async Task SelectBestCandidate_Fallback_ToEndpointWithoutConstraints() - { - // Arrange - var nomatch1 = CreateEndpoint( - "nomatch1", - new BooleanConstraint() { Pass = true, Order = 0, }, - new BooleanConstraint() { Pass = true, Order = 1, }, - new BooleanConstraint() { Pass = false, Order = 2, }); - - var nomatch2 = CreateEndpoint( - "nomatch2", - new BooleanConstraint() { Pass = true, Order = 0, }, - new BooleanConstraint() { Pass = true, Order = 1, }, - new BooleanConstraint() { Pass = false, Order = 3, }); - - var best = CreateEndpoint("best"); - - var endpoints = new[] { best, nomatch1, nomatch2 }; - - var selector = CreateSelector(endpoints); - var httpContext = CreateHttpContext("POST"); - var feature = new EndpointFeature(); - - // Act - await selector.SelectAsync(httpContext, feature, CreateCandidateSet(endpoints)); - - // Assert - Assert.Same(best, feature.Endpoint); - } - - private static MatcherEndpoint CreateEndpoint(string displayName, params object[] metadata) - { - return new MatcherEndpoint( - MatcherEndpoint.EmptyInvoker, - RoutePatternFactory.Parse("/"), - new RouteValueDictionary(), - 0, - new EndpointMetadataCollection(metadata), - displayName); - } - - private static CandidateSet CreateCandidateSet(MatcherEndpoint[] endpoints) - { - var scores = new int[endpoints.Length]; - return new CandidateSet(endpoints, scores); - } - - private static EndpointSelector CreateSelector(IReadOnlyList actions, ILoggerFactory loggerFactory = null) - { - loggerFactory = loggerFactory ?? NullLoggerFactory.Instance; - - var endpointDataSource = new CompositeEndpointDataSource(new[] { new DefaultEndpointDataSource(actions) }); - - var actionConstraintProviders = new IEndpointConstraintProvider[] { - new DefaultEndpointConstraintProvider(), - new BooleanConstraintProvider(), - }; - - return new EndpointConstraintEndpointSelector( - endpointDataSource, - GetEndpointConstraintCache(actionConstraintProviders), - loggerFactory); - } - - private static HttpContext CreateHttpContext(string httpMethod) - { - var serviceProvider = new ServiceCollection().BuildServiceProvider(); - - var httpContext = new Mock(MockBehavior.Strict); - - var request = new Mock(MockBehavior.Strict); - request.SetupGet(r => r.Method).Returns(httpMethod); - request.SetupGet(r => r.Path).Returns(new PathString("/test")); - request.SetupGet(r => r.Headers).Returns(new HeaderDictionary()); - httpContext.SetupGet(c => c.Request).Returns(request.Object); - httpContext.SetupGet(c => c.RequestServices).Returns(serviceProvider); - - return httpContext.Object; - } - - private static EndpointConstraintCache GetEndpointConstraintCache(IEndpointConstraintProvider[] actionConstraintProviders = null) - { - return new EndpointConstraintCache( - new CompositeEndpointDataSource(Array.Empty()), - actionConstraintProviders.AsEnumerable() ?? new List()); - } - - private class BooleanConstraint : IEndpointConstraint - { - public bool Pass { get; set; } - - public int Order { get; set; } - - public bool Accept(EndpointConstraintContext context) - { - return Pass; - } - } - - private class ConstraintFactory : IEndpointConstraintFactory - { - public IEndpointConstraint Constraint { get; set; } - - public bool IsReusable => true; - - public IEndpointConstraint CreateInstance(IServiceProvider services) - { - return Constraint; - } - } - - private class BooleanConstraintMarker : IEndpointConstraintMetadata - { - public bool Pass { get; set; } - } - - private class BooleanConstraintProvider : IEndpointConstraintProvider - { - public int Order { get; set; } - - public void OnProvidersExecuting(EndpointConstraintProviderContext context) - { - foreach (var item in context.Results) - { - if (item.Metadata is BooleanConstraintMarker marker) - { - Assert.Null(item.Constraint); - item.Constraint = new BooleanConstraint() { Pass = marker.Pass }; - } - } - } - - public void OnProvidersExecuted(EndpointConstraintProviderContext context) - { - } - } - } -} diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultEndpointSelectorTest.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultEndpointSelectorTest.cs index 13d6ff86e9..a1a71cccd8 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultEndpointSelectorTest.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/DefaultEndpointSelectorTest.cs @@ -4,6 +4,7 @@ using System.Threading.Tasks; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Routing.Patterns; +using Moq; using Xunit; namespace Microsoft.AspNetCore.Routing.Matchers @@ -174,6 +175,37 @@ test: /test3", ex.Message); Assert.Null(feature.Endpoint); } + [Fact] + public async Task SelectAsync_RunsEndpointSelectorPolicies() + { + // Arrange + var endpoints = new MatcherEndpoint[] { CreateEndpoint("/test1"), CreateEndpoint("/test2"), CreateEndpoint("/test3"), }; + var scores = new int[] { 0, 0, 1 }; + var candidateSet = CreateCandidateSet(endpoints, scores); + + var policy = new Mock(); + policy + .As() + .Setup(p => p.Apply(It.IsAny(), It.IsAny())) + .Callback((c, cs) => + { + cs[1].IsValidCandidate = false; + }); + + candidateSet[0].IsValidCandidate = false; + candidateSet[1].IsValidCandidate = true; + candidateSet[2].IsValidCandidate = true; + + var (httpContext, feature) = CreateContext(); + var selector = CreateSelector(policy.Object); + + // Act + await selector.SelectAsync(httpContext, feature, candidateSet); + + // Assert + Assert.Same(endpoints[2], feature.Endpoint); + } + private static (HttpContext httpContext, IEndpointFeature feature) CreateContext() { return (new DefaultHttpContext(), new EndpointFeature()); @@ -195,9 +227,9 @@ test: /test3", ex.Message); return new CandidateSet(endpoints, scores); } - private static DefaultEndpointSelector CreateSelector() + private static DefaultEndpointSelector CreateSelector(params MatcherPolicy[] policies) { - return new DefaultEndpointSelector(); + return new DefaultEndpointSelector(policies); } } } diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/RouteMatcherBuilder.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/RouteMatcherBuilder.cs index a57eea4387..a1c621826f 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/RouteMatcherBuilder.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/RouteMatcherBuilder.cs @@ -5,9 +5,7 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; -using Microsoft.AspNetCore.Routing.EndpointConstraints; using Microsoft.AspNetCore.Routing.Patterns; -using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; namespace Microsoft.AspNetCore.Routing.Matchers @@ -30,10 +28,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers public override Matcher Build() { - var cache = new EndpointConstraintCache( - new CompositeEndpointDataSource(Array.Empty()), - new[] { new DefaultEndpointConstraintProvider(), }); - var selector = new EndpointConstraintEndpointSelector(null, cache, NullLoggerFactory.Instance); + var selector = new DefaultEndpointSelector(Array.Empty()); var groups = _endpoints .GroupBy(e => (e.Order, e.RoutePattern.InboundPrecedence, e.RoutePattern.RawText)) diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeRouterMatcherBuilder.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeRouterMatcherBuilder.cs index 949b0a7df2..6b19d353cf 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeRouterMatcherBuilder.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeRouterMatcherBuilder.cs @@ -5,7 +5,6 @@ using System; using System.Collections.Generic; using System.Linq; using System.Threading.Tasks; -using Microsoft.AspNetCore.Routing.EndpointConstraints; using Microsoft.AspNetCore.Routing.Internal; using Microsoft.AspNetCore.Routing.Template; using Microsoft.AspNetCore.Routing.Tree; @@ -36,10 +35,7 @@ namespace Microsoft.AspNetCore.Routing.Matchers new DefaultObjectPool(new UriBuilderContextPooledObjectPolicy()), new DefaultInlineConstraintResolver(Options.Create(new RouteOptions()))); - var cache = new EndpointConstraintCache( - new CompositeEndpointDataSource(Array.Empty()), - new[] { new DefaultEndpointConstraintProvider(), }); - var selector = new EndpointConstraintEndpointSelector(null, cache, NullLoggerFactory.Instance); + var selector = new DefaultEndpointSelector(Array.Empty()); var groups = _endpoints .GroupBy(e => (e.Order, e.RoutePattern.InboundPrecedence, e.RoutePattern.RawText))