Fix ArgumentNullException in ApiVersionActionSelector

src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/ApiVersionActionSelector.cs

@@ -13,7 +13,6 @@
     using System.Collections.Generic;
     using System.Diagnostics.Contracts;
     using System.Linq;
-    using System.Threading.Tasks;
     using static ApiVersion;
     using static System.Environment;
     using static System.String;
@@ -24,6 +23,7 @@
     [CLSCompliant( false )]
     public class ApiVersionActionSelector : IActionSelector
     {
+        private static readonly IReadOnlyList<ActionDescriptor> NoMatches = new ActionDescriptor[0];
         private readonly IActionSelectorDecisionTreeProvider decisionTreeProvider;
         private readonly ActionConstraintCache actionConstraintCache;
         private readonly IOptions<ApiVersioningOptions> options;
@@ -111,6 +111,7 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead
             else if ( finalMatches.Count == 1 )
             {
                 var selectedAction = finalMatches[0];
+                selectedAction.AggregateAllVersions( selectionContext );
                 httpContext.SetRequestedApiVersion( selectionContext.RequestedVersion );
                 return selectedAction;
             }
@@ -172,16 +173,11 @@ where ActionIsSatisfiedBy( action, model, context.RequestedVersion, implicitMatc
                 return bestMatches;
             }
 
-            var match = bestMatches[0];
-
-            if ( match.IsApiVersionNeutral() )
+            if ( bestMatches[0].IsApiVersionNeutral() )
             {
                 bestMatches.AddRange( implicitMatches );
-                return bestMatches;
             }
 
-            match.AggregateAllVersions( context );
-
             return bestMatches;
         }
 
@@ -281,6 +277,7 @@ private IReadOnlyList<ActionDescriptor> EvaluateActionConstraints( RouteContext
         {
             Contract.Requires( context != null );
             Contract.Requires( actions != null );
+            Contract.Ensures( Contract.Result<IReadOnlyList<ActionDescriptor>>() != null );
 
             var candidates = new List<ActionSelectorCandidate>();
 
@@ -292,22 +289,13 @@ private IReadOnlyList<ActionDescriptor> EvaluateActionConstraints( RouteContext
             }
 
             var matches = EvaluateActionConstraintsCore( context, candidates, startingOrder: null );
-            var results = default( List<ActionDescriptor> );
 
             if ( matches == null )
             {
-                return results;
-            }
-
-            results = new List<ActionDescriptor>( matches.Count );
-
-            for ( var i = 0; i < matches.Count; i++ )
-            {
-                var candidate = matches[i];
-                results.Add( candidate.Action );
+                return NoMatches;
             }
 
-            return results;
+            return matches.Select( candidate => candidate.Action ).ToArray();
         }
 
         private IReadOnlyList<ActionSelectorCandidate> EvaluateActionConstraintsCore( RouteContext context, IReadOnlyList<ActionSelectorCandidate> candidates, int? startingOrder )

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/ByNamespace/Controllers/V1/AgreementsController.cs

@@ -5,6 +5,7 @@
     [ApiVersion( "1.0" )]
     public class AgreementsController : Controller
     {
+        [HttpGet]
         public IActionResult Get( string accountId ) => Ok( new Agreement( GetType().FullName, accountId, HttpContext.GetRequestedApiVersion().ToString() ) );
     }
 }

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/ByNamespace/Controllers/V2/AgreementsController.cs

@@ -5,6 +5,7 @@
     [ApiVersion( "2.0" )]
     public class AgreementsController : Controller
     {
+        [HttpGet]
         public IActionResult Get( string accountId ) => Ok( new Agreement( GetType().FullName, accountId, HttpContext.GetRequestedApiVersion().ToString() ) );
     }
 }

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/ByNamespace/Controllers/V3/AgreementsController.cs

@@ -5,6 +5,7 @@
     [ApiVersion( "3.0" )]
     public class AgreementsController : Controller
     {
+        [HttpGet]
         public IActionResult Get( string accountId ) => Ok( new Agreement( GetType().FullName, accountId, HttpContext.GetRequestedApiVersion().ToString() ) );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AmbiguousController.cs

@@ -6,6 +6,7 @@
     [ApiVersion( "1.0" )]
     public sealed class AmbiguousController : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AmbiguousNeutralController.cs

@@ -7,6 +7,7 @@
     [ControllerName( "Ambiguous" )]
     public sealed class AmbiguousNeutralController : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AmbiguousToo2Controller.cs

@@ -7,6 +7,7 @@
     [ControllerName( "AmbiguousToo" )]
     public sealed class AmbiguousToo2Controller : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AmbiguousTooController.cs

@@ -6,6 +6,7 @@
     [ApiVersion( "1.0" )]
     public sealed class AmbiguousTooController : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/ApiVersionedRoute2Controller.cs

@@ -8,9 +8,11 @@
     [Route( "api/v{version:apiVersion}/attributed" )]
     public sealed class ApiVersionedRoute2Controller : Controller
     {
+        [HttpGet]
         [MapToApiVersion( "4.0" )]
         public Task<string> GetV4() => Task.FromResult( "Test" );
-
+
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/ApiVersionedRouteController.cs

@@ -9,6 +9,7 @@
     [Route( "api/v{version:apiVersion}/attributed" )]
     public sealed class ApiVersionedRouteController : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AttributeRoutedAmbiguous2Controller.cs

@@ -7,6 +7,7 @@
     [Route( "api/attributed/ambiguous" )]
     public sealed class AttributeRoutedAmbiguous2Controller : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AttributeRoutedAmbiguous3Controller.cs

@@ -7,6 +7,7 @@
     [Route( "api/attributed/ambiguous" )]
     public sealed class AttributeRoutedAmbiguous3Controller : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AttributeRoutedAmbiguousController.cs

@@ -7,6 +7,7 @@
     [Route( "api/attributed-ambiguous" )]
     public sealed class AttributeRoutedAmbiguousController : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AttributeRoutedAmbiguousNeutralController.cs

@@ -7,6 +7,7 @@
     [Route( "api/attributed-ambiguous" )]
     public sealed class AttributeRoutedAmbiguousNeutralController : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AttributeRoutedTest2Controller.cs

@@ -9,8 +9,10 @@
     [Route( "api/attributed" )]
     public sealed class AttributeRoutedTest2Controller : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
 
+        [HttpGet]
         [MapToApiVersion( "3.0" )]
         public Task<string> GetV3() => Task.FromResult( "Test" );
     }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AttributeRoutedTest4Controller.cs

@@ -9,6 +9,7 @@
     [Route( "api/attributed" )]
     public sealed class AttributeRoutedTest4Controller : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AttributeRoutedTestController.cs

@@ -6,6 +6,7 @@
     [Route( "api/attributed" )]
     public sealed class AttributeRoutedTestController : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/AttributeRoutedVersionNeutralController.cs

@@ -7,6 +7,7 @@
     [Route( "api/attributed-neutral" )]
     public sealed class AttributeRoutedVersionNeutralController : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/ConventionsController.cs

@@ -6,11 +6,13 @@
     [Route( "api/[controller]" )]
     public sealed class ConventionsController : Controller
     {
+        [HttpGet]
         public string Get() => "Test (1.0)";
 
         [HttpGet( "{id:int}" )]
         public string Get( int id ) => $"Test {id} (1.0)";
 
+        [HttpGet]
         public string GetV2() => "Test (2.0)";
 
         [HttpGet( "{id:int}" )]

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/NeutralController.cs

@@ -6,6 +6,7 @@
     [ApiVersionNeutral]
     public sealed class NeutralController : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/TestController.cs

@@ -5,6 +5,7 @@
 
     public sealed class TestController : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
     }
 }

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Simulators/TestVersion2Controller.cs

@@ -10,8 +10,10 @@
     [ControllerName( "Test" )]
     public sealed class TestVersion2Controller : Controller
     {
+        [HttpGet]
         public Task<string> Get() => Task.FromResult( "Test" );
 
+        [HttpGet]
         [MapToApiVersion( "3.0-Alpha" )]
         [MapToApiVersion( "3.0" )]
         public Task<string> Get3() => Task.FromResult( "Test" );

test/Microsoft.AspNetCore.Mvc.Versioning.Tests/Versioning/ApiVersionActionSelectorTest.cs

@@ -12,11 +12,13 @@
     using Simulators;
     using System;
     using System.Linq;
+    using System.Net.Http;
     using System.Reflection;
     using System.Threading.Tasks;
     using Xunit;
     using static ApiVersion;
     using static System.Environment;
+    using static System.Net.Http.HttpMethod;
     using static System.Net.HttpStatusCode;
 
     public partial class ApiVersionActionSelectorTest
@@ -173,6 +175,22 @@ public async Task select_best_candidate_should_return_404_for_unmatched_controll
             }
         }
 
+        [Fact]
+        public async Task select_best_candidate_should_return_404_for_unmatched_action()
+        {
+            // arrange
+            var request = new HttpRequestMessage( Post, "api/attributed?api-version=1.0" );
+
+            using ( var server = new WebServer() )
+            {
+                // act
+                var response = await server.Client.SendAsync( request );
+
+                // assert
+                response.StatusCode.Should().Be( NotFound );
+            }
+        }
+
         [Fact]
         public async Task select_best_candidate_should_assume_1X2E0_for_attributeX2Dbased_controller_when_allowed()
         {