Handle ArgumentNullException within ApiVersionActionSelector when no actions are matched and return HTTP 404 instead. Fixes #52. (#56)

Author: commonsensesoftware

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" );
     }
 }