Validate candidates before matching. Fixes #600

Author: Chris Martinez

src/Microsoft.AspNetCore.Mvc.Versioning/Routing/ApiVersionMatcherPolicy.cs

@@ -95,33 +95,28 @@ public Task ApplyAsync( HttpContext httpContext, CandidateSet candidates )
                 httpContext.Features.Get<IApiVersioningFeature>().RequestedApiVersion = apiVersion;
             }
 
-            var finalMatches = EvaluateApiVersion( candidates, apiVersion );
-
-            if ( finalMatches.Count == 0 )
+            if ( !MatchesApiVersion( candidates, apiVersion ) )
             {
                 httpContext.SetEndpoint( ClientError( httpContext, candidates ) );
             }
-            else
-            {
-                for ( var i = 0; i < finalMatches.Count; i++ )
-                {
-                    var (index, _, valid) = finalMatches[i];
-                    candidates.SetValidity( index, valid );
-                }
-            }
 
             return CompletedTask;
         }
 
-        static IReadOnlyList<(int Index, ActionDescriptor Action, bool Valid)> EvaluateApiVersion( CandidateSet candidates, ApiVersion? apiVersion )
+        static bool MatchesApiVersion( CandidateSet candidates, ApiVersion? apiVersion )
         {
-            var bestMatches = new List<(int Index, ActionDescriptor Action, bool)>();
-            var implicitMatches = new List<(int, ActionDescriptor, bool)>();
+            var bestMatches = new List<int>();
+            var implicitMatches = new List<int>();
 
             for ( var i = 0; i < candidates.Count; i++ )
             {
+                if ( !candidates.IsValidCandidate( i ) )
+                {
+                    continue;
+                }
+
                 ref var candidate = ref candidates[i];
-                var action = candidate.Endpoint.Metadata?.GetMetadata<ActionDescriptor>();
+                var action = candidate.Endpoint.Metadata.GetMetadata<ActionDescriptor>();
 
                 if ( action == null )
                 {
@@ -134,10 +129,10 @@ public Task ApplyAsync( HttpContext httpContext, CandidateSet candidates )
                 switch ( action.MappingTo( apiVersion ) )
                 {
                     case Explicit:
-                        bestMatches.Add( (i, action, candidates.IsValidCandidate( i )) );
+                        bestMatches.Add( i );
                         break;
                     case Implicit:
-                        implicitMatches.Add( (i, action, candidates.IsValidCandidate( i )) );
+                        implicitMatches.Add( i );
                         break;
                 }
 
@@ -149,20 +144,39 @@ public Task ApplyAsync( HttpContext httpContext, CandidateSet candidates )
             switch ( bestMatches.Count )
             {
                 case 0:
-                    bestMatches.AddRange( implicitMatches );
-                    break;
+                    if ( implicitMatches.Count == 0 )
+                    {
+                        return false;
+                    }
+
+                    for ( var i = 0; i < implicitMatches.Count; i++ )
+                    {
+                        candidates.SetValidity( implicitMatches[i], true );
+                    }
+
+                    return true;
                 case 1:
-                    var model = bestMatches[0].Action.GetApiVersionModel();
+                    ref var candidate = ref candidates[bestMatches[0]];
+                    var action = candidate.Endpoint.Metadata.GetMetadata<ActionDescriptor>();
+                    var model = action.GetApiVersionModel();
 
                     if ( model.IsApiVersionNeutral )
                     {
-                        bestMatches.AddRange( implicitMatches );
+                        for ( var i = 0; i < implicitMatches.Count; i++ )
+                        {
+                            candidates.SetValidity( implicitMatches[i], true );
+                        }
                     }
 
                     break;
             }
 
-            return bestMatches.ToArray();
+            for ( var i = 0; i < bestMatches.Count; i++ )
+            {
+                candidates.SetValidity( bestMatches[i], true );
+            }
+
+            return true;
         }
 
         bool IsRequestedApiVersionAmbiguous( HttpContext httpContext, out ApiVersion? apiVersion )
@@ -195,8 +209,13 @@ ApiVersion TrySelectApiVersion( HttpContext httpContext, CandidateSet candidates
 
             for ( var i = 0; i < candidates.Count; i++ )
             {
+                if ( !candidates.IsValidCandidate( i ) )
+                {
+                    continue;
+                }
+
                 ref var candidate = ref candidates[i];
-                var model = candidate.Endpoint.Metadata?.GetMetadata<ActionDescriptor>()?.GetApiVersionModel();
+                var model = candidate.Endpoint.Metadata.GetMetadata<ActionDescriptor>()?.GetApiVersionModel();
 
                 if ( model != null )
                 {

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Mvc/Basic/given a versioned Controller/when using a query string and split into two types.cs

@@ -61,17 +61,16 @@ public async Task then_get_with_integer_id_should_return_200()
         }
 
         [Fact]
-        public async Task then_get_returns_400_or_405_with_invalid_id()
+        public async Task then_get_returns_400_with_invalid_id()
         {
             // arrange
             var requestUrl = "api/values/abc?api-version=2.0";
-            var statusCode = UsingEndpointRouting ? NotFound : BadRequest;
 
             // act
             var response = await GetAsync( requestUrl );
 
             // assert
-            response.StatusCode.Should().Be( statusCode );
+            response.StatusCode.Should().Be( BadRequest );
         }
 
         [Theory]

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Mvc/Basic/given a versioned Controller/when using a url segment.cs

@@ -65,17 +65,16 @@ public async Task then_post_should_return_201( string version )
         }
 
         [Fact]
-        public async Task then_get_returns_400_or_405_with_invalid_id()
+        public async Task then_get_returns_400_with_invalid_id()
         {
             // arrange
             var requestUrl = "api/v2/helloworld/abc";
-            var statusCode = UsingEndpointRouting ? NotFound : BadRequest;
 
             // act
             var response = await GetAsync( requestUrl );
 
-            // assert
-            response.StatusCode.Should().Be( statusCode );
+            // asserts
+            response.StatusCode.Should().Be( BadRequest );
         }
 
         [Theory]