From 28d1bc4fa209d7c751f3d7ae18c36b445649e249 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 12 Jun 2018 15:47:34 +1200 Subject: [PATCH] Fix TreeMatcher's use of EndpointSelector (#551) --- .../Matchers/TreeMatcher.cs | 50 ++++++++----------- .../Matchers/TreeMatcherTests.cs | 35 ++++++++++++- 2 files changed, 55 insertions(+), 30 deletions(-) diff --git a/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs b/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs index 1e93f17..36ae3b8 100644 --- a/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs +++ b/src/Microsoft.AspNetCore.Routing/Matchers/TreeMatcher.cs @@ -141,37 +141,31 @@ namespace Microsoft.AspNetCore.Routing.Matchers private Task SelectEndpointAsync(HttpContext httpContext, IEndpointFeature feature, IReadOnlyList endpoints) { - var bestEndpoint = _endpointSelector.SelectBestCandidate(httpContext, endpoints); + MatcherEndpoint endpoint; - // REVIEW: Note that this code doesn't do anything significant now. This will eventually incorporate something like IActionConstraint - switch (endpoints.Count) + try { - case 0: - { - Log.MatchFailed(_logger, httpContext); - return Task.CompletedTask; - } - - case 1: - { - var endpoint = endpoints[0]; - Log.MatchSuccess(_logger, httpContext, endpoint); - - feature.Endpoint = endpoint; - feature.Invoker = endpoint.Invoker; - - return Task.CompletedTask; - } - - default: - { - Log.MatchAmbiguous(_logger, httpContext, endpoints); - var message = Resources.FormatAmbiguousEndpoints( - Environment.NewLine, - string.Join(Environment.NewLine, endpoints.Select(a => a.DisplayName))); - throw new AmbiguousMatchException(message); - } + endpoint = (MatcherEndpoint)_endpointSelector.SelectBestCandidate(httpContext, endpoints); } + catch (Exception ex) + { + // Handle AmbiguousMatchException from EndpointSelector + return Task.FromException(ex); + } + + if (endpoint == null) + { + Log.MatchFailed(_logger, httpContext); + } + else + { + Log.MatchSuccess(_logger, httpContext, endpoint); + + feature.Endpoint = endpoint; + feature.Invoker = endpoint.Invoker; + } + + return Task.CompletedTask; } private UrlMatchingTree[] CreateTrees(IReadOnlyList endpoints) diff --git a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs index 8bebe11..fa1c78a 100644 --- a/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs +++ b/test/Microsoft.AspNetCore.Routing.Tests/Matchers/TreeMatcherTests.cs @@ -15,9 +15,9 @@ namespace Microsoft.AspNetCore.Routing.Matchers { public class TreeMatcherTests { - private MatcherEndpoint CreateEndpoint(string template, int order, object values = null) + private MatcherEndpoint CreateEndpoint(string template, int order, object values = null, EndpointMetadataCollection metadata = null) { - return new MatcherEndpoint((next) => null, template, values, order, EndpointMetadataCollection.Empty, template); + return new MatcherEndpoint((next) => null, template, values, order, metadata ?? EndpointMetadataCollection.Empty, template); } private TreeMatcher CreateTreeMatcher(EndpointDataSource endpointDataSource) @@ -58,5 +58,36 @@ namespace Microsoft.AspNetCore.Routing.Matchers // Assert Assert.Equal(lowerOrderEndpoint, endpointFeature.Endpoint); } + + [Fact] + public async Task MatchAsync_MultipleMatches_EndpointSelectorCalled() + { + // Arrange + var endpointWithoutConstraint = CreateEndpoint("/Teams", 0); + var endpointWithConstraint = CreateEndpoint( + "/Teams", + 0, + metadata: new EndpointMetadataCollection(new object[] { new HttpMethodEndpointConstraint(new[] { "POST" }) })); + + var endpointDataSource = new DefaultEndpointDataSource(new List + { + endpointWithoutConstraint, + endpointWithConstraint + }); + + var treeMatcher = CreateTreeMatcher(endpointDataSource); + + var httpContext = new DefaultHttpContext(); + httpContext.Request.Method = "POST"; + httpContext.Request.Path = "/Teams"; + + var endpointFeature = new EndpointFeature(); + + // Act + await treeMatcher.MatchAsync(httpContext, endpointFeature); + + // Assert + Assert.Equal(endpointWithConstraint, endpointFeature.Endpoint); + } } }