Updates the matcher policy to weigh explicit versus implicit matches without considering the score and invalidating candidates that are still valid. Fixes #1101

Author: commonsensesoftware

src/AspNetCore/Acceptance/Asp.Versioning.Mvc.Acceptance.Tests/Mvc/UsingAttributes/Controllers/OverlappingRouteTemplateController.cs

@@ -5,7 +5,9 @@
 
 namespace Asp.Versioning.Mvc.UsingAttributes.Controllers;
 
+using Microsoft.AspNetCore.Http;
 using Microsoft.AspNetCore.Mvc;
+using System.Net.Mime;
 
 [ApiController]
 [ApiVersion( "1.0" )]
@@ -31,4 +33,13 @@ public class OverlappingRouteTemplateController : ControllerBase
     [HttpGet( "[action]/{id}" )]
     [MapToApiVersion( "1.0" )]
     public string Echo( string id ) => id;
+
+    [HttpGet]
+    [ProducesResponseType( StatusCodes.Status200OK )]
+    public IActionResult Get() => Ok();
+
+    [HttpPost]
+    [Consumes( MediaTypeNames.Application.Json )]
+    [ProducesResponseType( StatusCodes.Status201Created )]
+    public IActionResult Post( [FromBody] string body ) => CreatedAtAction( nameof( Get ), body );
 }

src/AspNetCore/Acceptance/Asp.Versioning.Mvc.Acceptance.Tests/Mvc/UsingAttributes/given a versioned Controller/when two route templates overlap.cs

@@ -86,6 +86,21 @@ public async Task then_route_with_same_score_and_different_versions_should_retur
         response.StatusCode.Should().Be( statusCode );
     }
 
+    [Fact]
+    public async Task then_route_with_different_scores_and_same_version_should_return_expected_status()
+    {
+        // arrange
+
+
+        // act
+        var get = await GetAsync( "api/v1/values" );
+        var post = await PostAsync( "api/v1/values", "test" );
+
+        // assert
+        get.StatusCode.Should().Be( OK );
+        post.StatusCode.Should().Be( Created );
+    }
+
     public when_two_route_templates_overlap( OverlappingRouteTemplateFixture fixture, ITestOutputHelper console )
         : base( fixture ) => console.WriteLine( fixture.DirectedGraphVisualizationUrl );
 }

src/AspNetCore/WebApi/src/Asp.Versioning.Http/Routing/ApiVersionMatcherPolicy.cs

@@ -372,14 +372,9 @@ private static void Collate(
     private static (bool Matched, bool HasCandidates) MatchApiVersion( CandidateSet candidates, ApiVersion? apiVersion )
     {
         var total = candidates.Count;
-        var count = 0;
-        var array = default( Match[] );
-        var bestMatch = default( Match? );
+        var matched = false;
+        var implicitMatches = new Matches( stackalloc int[total] );
         var hasCandidates = false;
-        Span<Match> matches =
-            total <= 16
-            ? stackalloc Match[total]
-            : ( array = ArrayPool<Match>.Shared.Rent( total ) ).AsSpan();
 
         for ( var i = 0; i < total; i++ )
         {
@@ -397,56 +392,38 @@ private static (bool Matched, bool HasCandidates) MatchApiVersion( CandidateSet
                 continue;
             }
 
-            var score = candidate.Score;
-            bool isExplicit;
-
-            // perf: always make the candidate invalid so we only need to loop through the
-            // final, best matches for any remaining candidates
-            candidates.SetValidity( i, false );
-
             switch ( metadata.MappingTo( apiVersion ) )
             {
                 case Explicit:
-                    isExplicit = true;
+                    matched = true;
                     break;
                 case Implicit:
-                    isExplicit = metadata.IsApiVersionNeutral;
+                    if ( metadata.IsApiVersionNeutral )
+                    {
+                        matched = true;
+                    }
+                    else
+                    {
+                        implicitMatches.Add( i );
+                    }
+
                     break;
                 default:
+                    candidates.SetValidity( i, false );
                     continue;
             }
-
-            var match = new Match( i, score, isExplicit );
-
-            matches[count++] = match;
-
-            if ( !bestMatch.HasValue || match.CompareTo( bestMatch.Value ) > 0 )
-            {
-                bestMatch = match;
-            }
         }
 
-        var matched = false;
-
-        if ( bestMatch.HasValue )
+        if ( matched )
         {
-            matched = true;
-            var match = bestMatch.Value;
-
-            for ( var i = 0; i < count; i++ )
+            for ( var i = 0; i < implicitMatches.Count; i++ )
             {
-                ref readonly var otherMatch = ref matches[i];
-
-                if ( match.CompareTo( otherMatch ) == 0 )
-                {
-                    candidates.SetValidity( otherMatch.Index, true );
-                }
+                candidates.SetValidity( implicitMatches[i], false );
             }
         }
-
-        if ( array is not null )
+        else
         {
-            ArrayPool<Match>.Shared.Return( array );
+            matched = !implicitMatches.IsEmpty;
         }
 
         return (matched, hasCandidates);
@@ -481,17 +458,18 @@ private ValueTask<ApiVersion> TrySelectApiVersionAsync( HttpContext httpContext,
     bool INodeBuilderPolicy.AppliesToEndpoints( IReadOnlyList<Endpoint> endpoints ) =>
         !ContainsDynamicEndpoints( endpoints ) && AppliesToEndpoints( endpoints );
 
-    private readonly struct Match( int index, int score, bool isExplicit )
+    private ref struct Matches( Span<int> indexes )
     {
-        internal readonly int Index = index;
-        internal readonly int Score = score;
-        internal readonly bool IsExplicit = isExplicit;
+        private readonly Span<int> indexes = indexes;
+        private int count;
 
-        internal int CompareTo( in Match other )
-        {
-            var result = -Score.CompareTo( other.Score );
-            return result == 0 ? IsExplicit.CompareTo( other.IsExplicit ) : result;
-        }
+        public readonly int this[int index] => indexes[index];
+
+        public readonly bool IsEmpty => count == 0;
+
+        public readonly int Count => count;
+
+        public void Add( int index ) => indexes[count++] = index;
     }
 
     private sealed class ApiVersionCollator(