Merge pull request #888 from dotnet-maestro-bot/merge/release/2.2-to-master

[automated] Merge branch 'release/2.2' => 'master'

benchmarks/Microsoft.AspNetCore.Routing.Performance/Matching/MatcherBuilderMultipleEntryBenchmark.cs

@@ -2,14 +2,25 @@
 // 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.Threading.Tasks;
 using BenchmarkDotNet.Attributes;
+using Microsoft.AspNetCore.Http;
+using Microsoft.AspNetCore.Routing.TestObjects;
 using Microsoft.Extensions.DependencyInjection;
+using Microsoft.Extensions.Logging;
+using Microsoft.Extensions.Logging.Abstractions;
+using Microsoft.Extensions.Options;
 
 namespace Microsoft.AspNetCore.Routing.Matching
 {
     public partial class MatcherBuilderMultipleEntryBenchmark : EndpointRoutingBenchmarkBase
     {
         private IServiceProvider _services;
+        private List<MatcherPolicy> _policies;
+        private ILoggerFactory _loggerFactory;
+        private DefaultEndpointSelector _selector;
+        private DefaultParameterPolicyFactory _parameterPolicyFactory;
 
         [GlobalSetup]
         public void Setup()
@@ -28,6 +39,26 @@ namespace Microsoft.AspNetCore.Routing.Matching
             Endpoints[8] = CreateEndpoint("/v2/account/{id}", "POST");
             Endpoints[9] = CreateEndpoint("/v2/account/{id}", "UPDATE");
 
+            // Define an unordered mixture of policies that implement INodeBuilderPolicy,
+            // IEndpointComparerPolicy and/or IEndpointSelectorPolicy
+            _policies = new List<MatcherPolicy>()
+                {
+                    CreateNodeBuilderPolicy(4),
+                    CreateUberPolicy(2),
+                    CreateNodeBuilderPolicy(3),
+                    CreateEndpointComparerPolicy(5),
+                    CreateNodeBuilderPolicy(1),
+                    CreateEndpointSelectorPolicy(9),
+                    CreateEndpointComparerPolicy(7),
+                    CreateNodeBuilderPolicy(6),
+                    CreateEndpointSelectorPolicy(10),
+                    CreateUberPolicy(12),
+                    CreateEndpointComparerPolicy(11)
+                };
+            _loggerFactory = NullLoggerFactory.Instance;
+            _selector = new DefaultEndpointSelector();
+            _parameterPolicyFactory = new DefaultParameterPolicyFactory(Options.Create(new RouteOptions()), new TestServiceProvider());
+
             _services = CreateServices();
         }
 
@@ -46,5 +77,132 @@ namespace Microsoft.AspNetCore.Routing.Matching
             var builder = _services.GetRequiredService<DfaMatcherBuilder>();
             SetupMatcher(builder);
         }
+
+        [Benchmark]
+        public void Constructor_Policies()
+        {
+            new DfaMatcherBuilder(_loggerFactory, _parameterPolicyFactory, _selector, _policies);
+        }
+
+        private static MatcherPolicy CreateNodeBuilderPolicy(int order)
+        {
+            return new TestNodeBuilderPolicy(order);
+        }
+        private static MatcherPolicy CreateEndpointComparerPolicy(int order)
+        {
+            return new TestEndpointComparerPolicy(order);
+        }
+
+        private static MatcherPolicy CreateEndpointSelectorPolicy(int order)
+        {
+            return new TestEndpointSelectorPolicy(order);
+        }
+
+        private static MatcherPolicy CreateUberPolicy(int order)
+        {
+            return new TestUberPolicy(order);
+        }
+
+        private class TestUberPolicy : TestMatcherPolicyBase, INodeBuilderPolicy, IEndpointComparerPolicy
+        {
+            public TestUberPolicy(int order) : base(order)
+            {
+            }
+
+            public IComparer<Endpoint> Comparer => new TestEndpointComparer();
+
+            public bool AppliesToEndpoints(IReadOnlyList<Endpoint> endpoints)
+            {
+                return false;
+            }
+
+            public PolicyJumpTable BuildJumpTable(int exitDestination, IReadOnlyList<PolicyJumpTableEdge> edges)
+            {
+                throw new NotImplementedException();
+            }
+
+            public IReadOnlyList<PolicyNodeEdge> GetEdges(IReadOnlyList<Endpoint> endpoints)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
+        private class TestNodeBuilderPolicy : TestMatcherPolicyBase, INodeBuilderPolicy
+        {
+            public TestNodeBuilderPolicy(int order) : base(order)
+            {
+            }
+
+            public bool AppliesToEndpoints(IReadOnlyList<Endpoint> endpoints)
+            {
+                return false;
+            }
+
+            public PolicyJumpTable BuildJumpTable(int exitDestination, IReadOnlyList<PolicyJumpTableEdge> edges)
+            {
+                throw new NotImplementedException();
+            }
+
+            public IReadOnlyList<PolicyNodeEdge> GetEdges(IReadOnlyList<Endpoint> endpoints)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
+        private class TestEndpointComparerPolicy : TestMatcherPolicyBase, IEndpointComparerPolicy
+        {
+            public TestEndpointComparerPolicy(int order) : base(order)
+            {
+            }
+
+            public IComparer<Endpoint> Comparer => new TestEndpointComparer();
+
+            public bool AppliesToEndpoints(IReadOnlyList<Endpoint> endpoints)
+            {
+                return false;
+            }
+
+            public Task ApplyAsync(HttpContext httpContext, EndpointSelectorContext context, CandidateSet candidates)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
+        private class TestEndpointSelectorPolicy : TestMatcherPolicyBase, IEndpointSelectorPolicy
+        {
+            public TestEndpointSelectorPolicy(int order) : base(order)
+            {
+            }
+
+            public bool AppliesToEndpoints(IReadOnlyList<Endpoint> endpoints)
+            {
+                return false;
+            }
+
+            public Task ApplyAsync(HttpContext httpContext, EndpointSelectorContext context, CandidateSet candidates)
+            {
+                throw new NotImplementedException();
+            }
+        }
+
+        private abstract class TestMatcherPolicyBase : MatcherPolicy
+        {
+            private int _order;
+
+            protected TestMatcherPolicyBase(int order)
+            {
+                _order = order;
+            }
+
+            public override int Order { get { return _order; } }
+        }
+
+        private class TestEndpointComparer : IComparer<Endpoint>
+        {
+            public int Compare(Endpoint x, Endpoint y)
+            {
+                return 0;
+            }
+        }
     }
 }

src/Microsoft.AspNetCore.Routing/Matching/DfaMatcherBuilder.cs

@@ -17,7 +17,7 @@ namespace Microsoft.AspNetCore.Routing.Matching
         private readonly ILoggerFactory _loggerFactory;
         private readonly ParameterPolicyFactory _parameterPolicyFactory;
         private readonly EndpointSelector _selector;
-        private readonly MatcherPolicy[] _policies;
+        private readonly IEndpointSelectorPolicy[] _endpointSelectorPolicies;
         private readonly INodeBuilderPolicy[] _nodeBuilders;
         private readonly EndpointComparer _comparer;
 
@@ -39,11 +39,11 @@ namespace Microsoft.AspNetCore.Routing.Matching
             _loggerFactory = loggerFactory;
             _parameterPolicyFactory = parameterPolicyFactory;
             _selector = selector;
-            _policies = policies.OrderBy(p => p.Order).ToArray();
 
-            // Taking care to use _policies, which has been sorted.
+            var (nodeBuilderPolicies, endpointComparerPolicies, endpointSelectorPolicies) = ExtractPolicies(policies.OrderBy(p => p.Order));
-            _nodeBuilders = _policies.OfType<INodeBuilderPolicy>().ToArray();
+            _endpointSelectorPolicies = endpointSelectorPolicies;
-            _comparer = new EndpointComparer(_policies.OfType<IEndpointComparerPolicy>().ToArray());
+            _nodeBuilders = nodeBuilderPolicies;
+            _comparer = new EndpointComparer(endpointComparerPolicies);
 
             _assignments = new Dictionary<string, int>(StringComparer.OrdinalIgnoreCase);
             _slots = new List<KeyValuePair<string, object>>();
@@ -383,10 +383,10 @@ namespace Microsoft.AspNetCore.Routing.Matching
             List<IEndpointSelectorPolicy> endpointSelectorPolicies = null;
             if (node.Matches?.Count > 0)
             {
-                for (var i = 0; i < _policies.Length; i++)
+                for (var i = 0; i < _endpointSelectorPolicies.Length; i++)
                 {
-                    if (_policies[i] is IEndpointSelectorPolicy endpointSelectorPolicy &&
+                    var endpointSelectorPolicy = _endpointSelectorPolicies[i];
-                        endpointSelectorPolicy.AppliesToEndpoints(node.Matches))
+                    if (endpointSelectorPolicy.AppliesToEndpoints(node.Matches))
                     {
                         if (endpointSelectorPolicies == null)
                         {
@@ -465,16 +465,16 @@ namespace Microsoft.AspNetCore.Routing.Matching
         // internal for tests
         internal Candidate CreateCandidate(Endpoint endpoint, int score)
         {
-            _assignments.Clear();
-            _slots.Clear();
-            _captures.Clear();
-            _complexSegments.Clear();
-            _constraints.Clear();
-
             (string parameterName, int segmentIndex, int slotIndex) catchAll = default;
 
             if (endpoint is RouteEndpoint routeEndpoint)
             {
+                _assignments.Clear();
+                _slots.Clear();
+                _captures.Clear();
+                _complexSegments.Clear();
+                _constraints.Clear();
+
                 foreach (var kvp in routeEndpoint.RoutePattern.Defaults)
                 {
                     _assignments.Add(kvp.Key, _assignments.Count);
@@ -539,16 +539,27 @@ namespace Microsoft.AspNetCore.Routing.Matching
                         }
                     }
                 }
-            }
 
-            return new Candidate(
+                return new Candidate(
-                endpoint,
+                    endpoint,
-                score,
+                    score,
-                _slots.ToArray(),
+                    _slots.ToArray(),
-                _captures.ToArray(),
+                    _captures.ToArray(),
-                catchAll,
+                    catchAll,
-                _complexSegments.ToArray(),
+                    _complexSegments.ToArray(),
-                _constraints.ToArray());
+                    _constraints.ToArray());
+            }
+            else
+            {
+                return new Candidate(
+                    endpoint,
+                    score,
+                    Array.Empty<KeyValuePair<string, object>>(),
+                    Array.Empty<(string parameterName, int segmentIndex, int slotIndex)>(),
+                    catchAll,
+                    Array.Empty<(RoutePatternPathSegment pathSegment, int segmentIndex)>(),
+                    Array.Empty<KeyValuePair<string, IRouteConstraint>>());
+            }
         }
 
         private int[] GetGroupLengths(DfaNode node)
@@ -682,5 +693,32 @@ namespace Microsoft.AspNetCore.Routing.Matching
                 work = nextWork;
             }
         }
+
+        private static (INodeBuilderPolicy[] nodeBuilderPolicies, IEndpointComparerPolicy[] endpointComparerPolicies, IEndpointSelectorPolicy[] endpointSelectorPolicies) ExtractPolicies(IEnumerable<MatcherPolicy> policies)
+        {
+            var nodeBuilderPolicies = new List<INodeBuilderPolicy>();
+            var endpointComparerPolicies = new List<IEndpointComparerPolicy>();
+            var endpointSelectorPolicies = new List<IEndpointSelectorPolicy>();
+
+            foreach (var policy in policies)
+            {
+                if (policy is INodeBuilderPolicy nodeBuilderPolicy)
+                {
+                    nodeBuilderPolicies.Add(nodeBuilderPolicy);
+                }
+
+                if (policy is IEndpointComparerPolicy endpointComparerPolicy)
+                {
+                    endpointComparerPolicies.Add(endpointComparerPolicy);
+                }
+
+                if (policy is IEndpointSelectorPolicy endpointSelectorPolicy)
+                {
+                    endpointSelectorPolicies.Add(endpointSelectorPolicy);
+                }
+            }
+
+            return (nodeBuilderPolicies.ToArray(), endpointComparerPolicies.ToArray(), endpointSelectorPolicies.ToArray());
+        }
     }
 }

src/Microsoft.AspNetCore.Routing/RouteValuesAddressScheme.cs

@@ -16,7 +16,7 @@ namespace Microsoft.AspNetCore.Routing
     {
         private readonly CompositeEndpointDataSource _dataSource;
         private LinkGenerationDecisionTree _allMatchesLinkGenerationTree;
-        private IDictionary<string, List<OutboundMatchResult>> _namedMatchResults;
+        private Dictionary<string, List<OutboundMatchResult>> _namedMatchResults;
 
         public RouteValuesAddressScheme(CompositeEndpointDataSource dataSource)
         {
@@ -33,7 +33,7 @@ namespace Microsoft.AspNetCore.Routing
 
         public IEnumerable<Endpoint> FindEndpoints(RouteValuesAddress address)
         {
-            IEnumerable<OutboundMatchResult> matchResults = null;
+            IList<OutboundMatchResult> matchResults = null;
             if (string.IsNullOrEmpty(address.RouteName))
             {
                 matchResults = _allMatchesLinkGenerationTree.GetMatches(
@@ -45,14 +45,33 @@ namespace Microsoft.AspNetCore.Routing
                 matchResults = namedMatchResults;
             }
 
-            if (matchResults == null || !matchResults.Any())
+            if (matchResults != null)
             {
-                return Array.Empty<Endpoint>();
+                var matchCount = matchResults.Count;
+                if (matchCount > 0)
+                {
+                    if (matchResults.Count == 1)
+                    {
+                        // Special case having a single result to avoid creating iterator state machine
+                        return new[] { (RouteEndpoint)matchResults[0].Match.Entry.Data };
+                    }
+                    else
+                    {
+                        // Use separate method since one cannot have regular returns in an iterator method
+                        return GetEndpoints(matchResults, matchCount);
+                    }
+                }
             }
 
-            return matchResults
+            return Array.Empty<Endpoint>();
-                .Select(matchResult => matchResult.Match)
+        }
-                .Select(match => (RouteEndpoint)match.Entry.Data);
+
+        private static IEnumerable<Endpoint> GetEndpoints(IList<OutboundMatchResult> matchResults, int matchCount)
+        {
+            for (var i = 0; i < matchCount; i++)
+            {
+                yield return (RouteEndpoint)matchResults[i].Match.Entry.Data;
+            }
         }
 
         private void HandleChange()
@@ -73,7 +92,7 @@ namespace Microsoft.AspNetCore.Routing
             // as refresh of new endpoints happens within a lock and also these fields are not publicly accessible.
             var (allMatches, namedMatchResults) = GetOutboundMatches();
             _namedMatchResults = namedMatchResults;
-            _allMatchesLinkGenerationTree = new LinkGenerationDecisionTree(allMatches.ToArray());
+            _allMatchesLinkGenerationTree = new LinkGenerationDecisionTree(allMatches);
         }
 
         /// Decision tree is built using the 'required values' of actions.
@@ -93,21 +112,25 @@ namespace Microsoft.AspNetCore.Routing
         ///     requiredValues: new { controller = "Orders", action = "GetById" },
         ///   A call to GetLink("OrdersApi", new { id = "10" }) cannot generate url as neither the supplied values or
         ///   current ambient values do not satisfy the decision tree that is built based on the required values.
-        protected virtual (IEnumerable<OutboundMatch>, IDictionary<string, List<OutboundMatchResult>>) GetOutboundMatches()
+        protected virtual (List<OutboundMatch>, Dictionary<string, List<OutboundMatchResult>>) GetOutboundMatches()
         {
             var allOutboundMatches = new List<OutboundMatch>();
             var namedOutboundMatchResults = new Dictionary<string, List<OutboundMatchResult>>(
                 StringComparer.OrdinalIgnoreCase);
 
-            var endpoints = _dataSource.Endpoints.OfType<RouteEndpoint>();
+            foreach (var endpoint in _dataSource.Endpoints)
-            foreach (var endpoint in endpoints)
             {
+                if (!(endpoint is RouteEndpoint routeEndpoint))
+                {
+                    continue;
+                }
+
                 if (endpoint.Metadata.GetMetadata<ISuppressLinkGenerationMetadata>()?.SuppressLinkGeneration == true)
                 {
                     continue;
                 }
 
-                var entry = CreateOutboundRouteEntry(endpoint);
+                var entry = CreateOutboundRouteEntry(routeEndpoint);
 
                 var outboundMatch = new OutboundMatch() { Entry = entry };
                 allOutboundMatches.Add(outboundMatch);
@@ -117,8 +140,7 @@ namespace Microsoft.AspNetCore.Routing
                     continue;
                 }
 
-                List<OutboundMatchResult> matchResults;
+                if (!namedOutboundMatchResults.TryGetValue(entry.RouteName, out var matchResults))
-                if (!namedOutboundMatchResults.TryGetValue(entry.RouteName, out matchResults))
                 {
                     matchResults = new List<OutboundMatchResult>();
                     namedOutboundMatchResults.Add(entry.RouteName, matchResults);

test/Microsoft.AspNetCore.Routing.Tests/RouteValueEqualityComparerTest.cs

@@ -0,0 +1,41 @@
+// 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 Xunit;
+
+namespace Microsoft.AspNetCore.Routing
+{
+    public class RouteValueEqualityComparerTest
+    {
+        private readonly RouteValueEqualityComparer _comparer;
+
+        public RouteValueEqualityComparerTest()
+        {
+            _comparer = new RouteValueEqualityComparer();
+        }
+
+        [Theory]
+        [InlineData(5, 7, false)]
+        [InlineData("foo", "foo", true)]
+        [InlineData("foo", "FoO", true)]
+        [InlineData("foo", "boo", false)]
+        [InlineData("7", 7, true)]
+        [InlineData(7, "7", true)]
+        [InlineData(5.7d, 5.7d, true)]
+        [InlineData(null, null, true)]
+        [InlineData(null, "foo", false)]
+        [InlineData("foo", null, false)]
+        [InlineData(null, "", true)]
+        [InlineData("", null, true)]
+        [InlineData("", "", true)]
+        [InlineData("", "foo", false)]
+        [InlineData("foo", "", false)]
+        [InlineData(true, true, true)]
+        [InlineData(true, false, false)]
+        public void EqualsTest(object x, object y, bool expected)
+        {
+            var actual = _comparer.Equals(x, y);
+            Assert.Equal(expected, actual);
+        }
+    }
+}

test/Microsoft.AspNetCore.Routing.Tests/RouteValuesAddressSchemeTest.cs

@@ -139,6 +139,94 @@ namespace Microsoft.AspNetCore.Routing
                 });
         }
 
+        [Fact]
+        public void FindEndpoints_LookedUpByCriteria_NoMatch()
+        {
+            // Arrange
+            var endpoint1 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { zipCode = 3510 },
+                requiredValues: new { id = 7 },
+                routeName: "OrdersApi");
+            var endpoint2 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { id = 12 },
+                requiredValues: new { zipCode = 3510 },
+                routeName: "OrdersApi");
+            var addressScheme = CreateAddressScheme(endpoint1, endpoint2);
+
+            // Act
+            var foundEndpoints = addressScheme.FindEndpoints(
+                new RouteValuesAddress
+                {
+                    ExplicitValues = new RouteValueDictionary(new { id = 8 }),
+                    AmbientValues = new RouteValueDictionary(new { urgent = false }),
+                });
+
+            // Assert
+            Assert.Empty(foundEndpoints);
+        }
+
+        [Fact]
+        public void FindEndpoints_LookedUpByCriteria_OneMatch()
+        {
+            // Arrange
+            var endpoint1 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { zipCode = 3510 },
+                requiredValues: new { id = 7 },
+                routeName: "OrdersApi");
+            var endpoint2 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { id = 12 },
+                routeName: "OrdersApi");
+            var addressScheme = CreateAddressScheme(endpoint1, endpoint2);
+
+            // Act
+            var foundEndpoints = addressScheme.FindEndpoints(
+                new RouteValuesAddress
+                {
+                    ExplicitValues = new RouteValueDictionary(new { id = 13 }),
+                    AmbientValues = new RouteValueDictionary(new { zipCode = 3500 }),
+                });
+
+            // Assert
+            var actual = Assert.Single(foundEndpoints);
+            Assert.Same(endpoint2, actual);
+        }
+
+        [Fact]
+        public void FindEndpoints_LookedUpByCriteria_MultipleMatches()
+        {
+            // Arrange
+            var endpoint1 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { zipCode = 3510 },
+                requiredValues: new { id = 7 },
+                routeName: "OrdersApi");
+            var endpoint2 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent}/{zipCode}",
+                defaults: new { id = 12 },
+                routeName: "OrdersApi");
+            var endpoint3 = CreateEndpoint(
+                "api/orders/{id}/{name?}/{urgent=true}/{zipCode}",
+                defaults: new { id = 12 },
+                routeName: "OrdersApi");
+            var addressScheme = CreateAddressScheme(endpoint1, endpoint2, endpoint3);
+
+            // Act
+            var foundEndpoints = addressScheme.FindEndpoints(
+                new RouteValuesAddress
+                {
+                    ExplicitValues = new RouteValueDictionary(new { id = 7 }),
+                    AmbientValues = new RouteValueDictionary(new { zipCode = 3500 }),
+                });
+
+            // Assert
+            Assert.Contains(endpoint1, foundEndpoints);
+            Assert.Contains(endpoint1, foundEndpoints);
+        }
+
         [Fact]
         public void FindEndpoints_ReturnsEndpoint_WhenLookedUpByRouteName()
         {
@@ -270,7 +358,7 @@ namespace Microsoft.AspNetCore.Routing
 
             public IDictionary<string, List<OutboundMatchResult>> NamedMatches { get; private set; }
 
-            protected override (IEnumerable<OutboundMatch>, IDictionary<string, List<OutboundMatchResult>>) GetOutboundMatches()
+            protected override (List<OutboundMatch>, Dictionary<string, List<OutboundMatchResult>>) GetOutboundMatches()
             {
                 var matches = base.GetOutboundMatches();
                 AllMatches = matches.Item1;