Refactor action selection to address 1.1 RC regression. Fixes #133. Fixes #130. Fixes #131. Resolves #134. Resolves #135.

Author: Chris Martinez

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

@@ -98,18 +98,24 @@ public DefaultApiVersionRoutePolicy(
         public virtual Task RouteAsync( RouteContext context )
         {
             var selectionResult = context.HttpContext.ApiVersionProperties().SelectionResult;
+            var match = selectionResult.BestMatch;
 
-            switch ( selectionResult.MatchingActions.Count )
+            if ( match == null )
             {
-                case 0:
-                    OnUnmatched( context, selectionResult );
-                    break;
-                case 1:
-                    OnSingleMatch( context, selectionResult, selectionResult.MatchingActions.First() );
-                    break;
-                default:
+                var hasAnyMatches = selectionResult.MatchingActions.SelectMany( i => i.Value ).Any();
+
+                if ( hasAnyMatches )
+                {
                     OnMultipleMatches( context, selectionResult );
-                    break;
+                }
+                else
+                {
+                    OnUnmatched( context, selectionResult );
+                }
+            }
+            else
+            {
+                OnSingleMatch( context, selectionResult, selectionResult.BestMatch );
             }
 
             return CompletedTask;
@@ -128,8 +134,9 @@ protected virtual void OnSingleMatch( RouteContext context, ActionSelectionResul
             Arg.NotNull( match, nameof( match ) );
 
             var handler = new DefaultActionHandler( ActionInvokerFactory, ActionContextAccessor, selectionResult, match );
+            var candidates = selectionResult.CandidateActions.SelectMany( kvp => kvp.Value );
 
-            match.Action.AggregateAllVersions( selectionResult.CandidateActions );
+            match.Action.AggregateAllVersions( candidates );
             context.Handler = handler.Invoke;
         }
 
@@ -157,7 +164,8 @@ protected virtual void OnMultipleMatches( RouteContext context, ActionSelectionR
             Arg.NotNull( context, nameof( context ) );
             Arg.NotNull( selectionResult, nameof( selectionResult ) );
 
-            var actionNames = Join( NewLine, selectionResult.MatchingActions.Select( match => match.Action.DisplayName ) );
+            var matchingActions = selectionResult.MatchingActions.OrderBy( kvp => kvp.Key ).SelectMany( kvp => kvp.Value ).Distinct();
+            var actionNames = Join( NewLine, matchingActions.Select( match => match.Action.DisplayName ) );
 
             Logger.AmbiguousActions( actionNames );
 
@@ -172,9 +180,9 @@ RequestHandler ClientError( RouteContext context, ActionSelectionResult selectio
             Contract.Requires( selectionResult != null );
 
             const RequestHandler NotFound = default( RequestHandler );
-            var candidates = selectionResult.CandidateActions;
+            var candidates = selectionResult.CandidateActions.OrderBy( kvp => kvp.Key ).SelectMany( kvp => kvp.Value ).Distinct().ToArray();
 
-            if ( candidates.Count == 0 )
+            if ( candidates.Length == 0 )
             {
                 return NotFound;
             }

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

@@ -3,24 +3,162 @@
     using Microsoft.AspNetCore.Mvc.Abstractions;
     using System;
     using System.Collections.Generic;
+    using System.Linq;
 
     /// <summary>
     /// Represents an API versioning action selection result for which a versioning policy can be applied.
     /// </summary>
     public class ActionSelectionResult
     {
+        readonly Dictionary<int, ICollection<ActionDescriptor>> candidateActions = new Dictionary<int, ICollection<ActionDescriptor>>();
+        readonly Dictionary<int, ICollection<ActionDescriptorMatch>> matchingActions = new Dictionary<int, ICollection<ActionDescriptorMatch>>();
+
+        /// <summary>
+        /// Gets the number of action selection iterations that have occurred.
+        /// </summary>
+        /// <value>The total number of action selection iterations that have occurred. The default value is zero.</value>
+        public int Iterations { get; private set; }
+
+        /// <summary>
+        /// Gets the best action descriptor match.
+        /// </summary>
+        /// <value>The best <see cref="ActionDescriptorMatch">action descriptor match</see> or <c>null</c>.</value>
+        /// <remarks>This property returns the first occurrence of a single match in the earliest iteration. If
+        /// no matches exist in any iteration or multiple matches exist, this property returns <c>null</c>.</remarks>
+        [CLSCompliant( false )]
+        public ActionDescriptorMatch BestMatch
+        {
+            get
+            {
+                foreach ( var iteration in matchingActions )
+                {
+                    switch ( iteration.Value.Count )
+                    {
+                        case 0:
+                            break;
+                        case 1:
+                            return iteration.Value.ElementAt( 0 );
+                        default:
+                            return null;
+                    }
+                }
+
+                return null;
+            }
+        }
+
+        /// <summary>
+        /// Gets a collection of candidate actions grouped by action selection iteration.
+        /// </summary>
+        /// <value>A <see cref="IReadOnlyDictionary{TKey, TValue}">read-only dictionary</see> of candidate
+        /// <see cref="ActionDescriptor">actions</see> per action selection iteration.</value>
+        [CLSCompliant( false )]
+        public IReadOnlyDictionary<int, ICollection<ActionDescriptor>> CandidateActions => candidateActions;
+
         /// <summary>
-        /// Gets the collection of all candidate controller actions for the current route.
+        /// Gets a collection of matching actions grouped by action selection iteration.
         /// </summary>
-        /// <value>A <see cref="ICollection{T}">collection</see> of <see cref="ActionDescriptor">actions</see> candidate actions for the current route.</value>
+        /// <value>A <see cref="IReadOnlyDictionary{TKey, TValue}">read-only dictionary</see> of
+        /// <see cref="ActionDescriptorMatch">matching actions</see> per action selection iteration.</value>
         [CLSCompliant( false )]
-        public ICollection<ActionDescriptor> CandidateActions { get; } = new HashSet<ActionDescriptor>();
+        public IReadOnlyDictionary<int, ICollection<ActionDescriptorMatch>> MatchingActions => matchingActions;
 
         /// <summary>
-        /// Gets the collection of controller actions matching the current route.
+        /// Adds the specified candidate actions to the selection result.
         /// </summary>
-        /// <value>A <see cref="ICollection{T}">collection</see> of <see cref="ActionDescriptorMatch">matching actions</see> for the current route.</value>
+        /// <param name="actions">The array of <see cref="ActionDescriptor">actions</see> to add to the selection result.</param>
         [CLSCompliant( false )]
-        public ICollection<ActionDescriptorMatch> MatchingActions { get; } = new HashSet<ActionDescriptorMatch>();
+        public void AddCandidates( params ActionDescriptor[] actions )
+        {
+            Arg.NotNull( actions, nameof( actions ) );
+
+            var key = Iterations;
+
+            if ( !candidateActions.TryGetValue( key, out var collection ) )
+            {
+                candidateActions[key] = collection = new HashSet<ActionDescriptor>();
+            }
+
+            collection.AddRange( actions );
+        }
+
+        /// <summary>
+        /// Adds the specified candidate actions to the selection result.
+        /// </summary>
+        /// <param name="actions">The <see cref="IEnumerable{T}">sequence</see> of <see cref="ActionDescriptor">actions</see>
+        /// to add to the selection result.</param>
+        [CLSCompliant( false )]
+        public void AddCandidates( IEnumerable<ActionDescriptor> actions )
+        {
+            Arg.NotNull( actions, nameof( actions ) );
+
+            var key = Iterations;
+
+            if ( !candidateActions.TryGetValue( key, out var collection ) )
+            {
+                candidateActions[key] = collection = new HashSet<ActionDescriptor>();
+            }
+
+            collection.AddRange( actions );
+        }
+
+        /// <summary>
+        /// Adds the specified matching actions to the selection result.
+        /// </summary>
+        /// <param name="matches">The array of <see cref="ActionDescriptorMatch">matching actions</see> to add to the selection result.</param>
+        [CLSCompliant( false )]
+        public void AddMatches( params ActionDescriptorMatch[] matches )
+        {
+            Arg.NotNull( matches, nameof( matches ) );
+
+            var key = Iterations;
+
+            if ( !matchingActions.TryGetValue( key, out var collection ) )
+            {
+                matchingActions[key] = collection = new HashSet<ActionDescriptorMatch>();
+            }
+
+            collection.AddRange( matches );
+        }
+
+        /// <summary>
+        /// Adds the specified matching actions to the selection result.
+        /// </summary>
+        /// <param name="matches">The <see cref="IEnumerable{T}">sequence</see> of <see cref="ActionDescriptorMatch">matching actions</see>
+        /// to add to the selection result.</param>
+        [CLSCompliant( false )]
+        public void AddMatches( IEnumerable<ActionDescriptorMatch> matches )
+        {
+            Arg.NotNull( matches, nameof( matches ) );
+
+            var key = Iterations;
+
+            if ( !matchingActions.TryGetValue( key, out var collection ) )
+            {
+                matchingActions[key] = collection = new HashSet<ActionDescriptorMatch>();
+            }
+
+            collection.AddRange( matches );
+        }
+
+        /// <summary>
+        /// Ends the current action selection iteration.
+        /// </summary>
+        public void EndIteration()
+        {
+            var key = Iterations;
+
+            if ( !candidateActions.ContainsKey( key ) )
+            {
+                candidateActions.Add( key, new ActionDescriptor[0] );
+            }
+
+            if ( !matchingActions.ContainsKey( key ) )
+            {
+                matchingActions.Add( key, new ActionDescriptorMatch[0] );
+            }
+
+            ++Iterations;
+        }
     }
 }

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

@@ -106,10 +106,11 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead
             var selectionResult = properties.SelectionResult;
 
             properties.ApiVersion = selectionContext.RequestedVersion;
-            selectionResult.CandidateActions.AddRange( candidates );
+            selectionResult.AddCandidates( candidates );
 
             if ( finalMatches == null )
             {
+                selectionResult.EndIteration();
                 return null;
             }
 
@@ -128,12 +129,12 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead
             if ( finalMatches.Count > 0 )
             {
                 var routeData = new RouteData( context.RouteData );
-                selectionResult.MatchingActions.AddRange( finalMatches.Select( action => new ActionDescriptorMatch( action, routeData ) ) );
+                selectionResult.AddMatches( finalMatches.Select( action => new ActionDescriptorMatch( action, routeData ) ) );
             }
 
-            // note: even though we may have had a successful match, this method could be called multiple times.
-            // the final decision is made by the IApiVersionRoutePolicy. we return here to make sure all candidates
-            // have been considered.
+            // note: even though we may have had a successful match, this method could be called multiple times. the final decision
+            // is made by the IApiVersionRoutePolicy. we return here to make sure all candidates have been considered.
+            selectionResult.EndIteration();
             return null;
         }
 

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/AcceptanceTest.cs

@@ -5,14 +5,20 @@
     using Builder;
     using Extensions.DependencyInjection;
     using Hosting;
+    using Microsoft.AspNetCore.Mvc.Razor;
+    using Microsoft.Extensions.PlatformAbstractions;
     using System;
     using System.Collections.Generic;
+    using System.IO;
+    using System.Linq;
     using System.Net.Http;
     using System.Reflection;
     using TestHost;
     using Versioning;
     using Xunit;
+    using static Microsoft.CodeAnalysis.MetadataReference;
     using static Microsoft.Extensions.DependencyInjection.ServiceDescriptor;
+    using static System.Reflection.Assembly;
 
     [Trait( "Framework", "ASP.NET Core" )]
     public abstract partial class AcceptanceTest : IDisposable
@@ -60,8 +66,9 @@ protected virtual void Dispose( bool disposing )
         TestServer CreateServer()
         {
             var builder = new WebHostBuilder()
-                .Configure( app => app.UseMvc( OnConfigureRoutes ).UseApiVersioning() )
-                .ConfigureServices( OnConfigureServices );
+                .Configure( app => app.UseMvc( OnConfigureRoutes ).UseMvcWithDefaultRoute().UseApiVersioning() )
+                .ConfigureServices( OnConfigureServices )
+                .UseContentRoot( GetContentRoot() );
 
             return new TestServer( builder );
         }
@@ -82,6 +89,33 @@ void OnConfigureServices( IServiceCollection services )
             services.Add( Singleton( partManager ) );
             services.AddMvc();
             services.AddApiVersioning( OnAddApiVersioning );
+            services.Configure<RazorViewEngineOptions>( options =>
+            {
+                options.CompilationCallback += context =>
+                {
+                    var assembly = GetType().GetTypeInfo().Assembly;
+                    var assemblies = assembly.GetReferencedAssemblies().Select( x => CreateFromFile( Load( x ).Location ) ).ToList();
+
+                    assemblies.Add( CreateFromFile( Load( new AssemblyName( "mscorlib" ) ).Location ) );
+                    assemblies.Add( CreateFromFile( Load( new AssemblyName( "System.Private.Corelib" ) ).Location ) );
+                    assemblies.Add( CreateFromFile( Load( new AssemblyName( "System.Dynamic.Runtime" ) ).Location ) );
+                    assemblies.Add( CreateFromFile( Load( new AssemblyName( "Microsoft.AspNetCore.Razor" ) ).Location ) );
+                    context.Compilation = context.Compilation.AddReferences( assemblies );
+                };
+            } );
+        }
+
+        string GetContentRoot()
+        {
+            var startupAssembly = GetType().GetTypeInfo().Assembly.GetName().Name;
+            var contentRoot = new DirectoryInfo( PlatformServices.Default.Application.ApplicationBasePath );
+
+            while ( contentRoot.Name != startupAssembly )
+            {
+                contentRoot = contentRoot.Parent;
+            }
+
+            return contentRoot.FullName;
         }
 
         protected abstract void OnAddApiVersioning( ApiVersioningOptions options );

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/BasicAcceptanceTest.cs

@@ -11,6 +11,8 @@ protected BasicAcceptanceTest()
             FilteredControllerTypes.Add( typeof( ValuesController ).GetTypeInfo() );
             FilteredControllerTypes.Add( typeof( Values2Controller ).GetTypeInfo() );
             FilteredControllerTypes.Add( typeof( HelloWorldController ).GetTypeInfo() );
+            FilteredControllerTypes.Add( typeof( HelloWorld2Controller ).GetTypeInfo() );
+            FilteredControllerTypes.Add( typeof( HomeController ).GetTypeInfo() );
         }
 
         protected override void OnAddApiVersioning( ApiVersioningOptions options ) => options.ReportApiVersions = true;

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/HelloWorld2Controller.cs

@@ -0,0 +1,23 @@
+namespace Microsoft.AspNetCore.Mvc.Basic.Controllers
+{
+    using AspNetCore.Routing;
+    using Microsoft.AspNetCore.Mvc;
+    using System;
+
+    [ApiVersion( "2.0" )]
+    [Route( "api/v{version:apiVersion}/HelloWorld" )]
+    public class HelloWorld2Controller : Controller
+    {
+        [HttpGet]
+        public IActionResult Get() => Ok( new { Controller = GetType().Name, Version = HttpContext.GetRequestedApiVersion().ToString() } );
+
+        [HttpGet( "{id:int}", Name = "GetMessageById-V2" )]
+        public IActionResult Get( int id ) => Ok( new { Controller = GetType().Name, Id = id, Version = HttpContext.GetRequestedApiVersion().ToString() } );
+
+        [HttpPost]
+        public IActionResult Post() => CreatedAtRoute( "GetMessageById-V2", new { id = 42 }, null );
+
+        [HttpGet( "search" )]
+        public IActionResult Search( string query ) => Ok( new { Controller = GetType().Name, Query = query, Version = HttpContext.GetRequestedApiVersion().ToString() } );
+    }
+}

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Basic/Controllers/HelloWorldController.cs

@@ -4,16 +4,20 @@
     using Microsoft.AspNetCore.Mvc;
     using System;
 
+    [ApiVersion( "1.0" )]
     [Route( "api/v{version:apiVersion}/[controller]" )]
     public class HelloWorldController : Controller
     {
         [HttpGet]
-        public IActionResult Get() => Ok( new { Controller = nameof( HelloWorldController ), Version = HttpContext.GetRequestedApiVersion().ToString() } );
+        public IActionResult Get() => Ok( new { Controller = GetType().Name, Version = HttpContext.GetRequestedApiVersion().ToString() } );
 
-        [HttpGet( "{id:int}", Name = "GetMessageById" )]
-        public IActionResult Get( int id ) => Ok( new { Controller = GetType().Name, Id = id, Version = HttpContext.GetRequestedApiVersion().ToString() } );
+        [HttpGet( "{id}", Name = "GetMessageById" )]
+        public IActionResult Get( string id ) => Ok( new { Controller = GetType().Name, Id = id, Version = HttpContext.GetRequestedApiVersion().ToString() } );
 
         [HttpPost]
         public IActionResult Post() => CreatedAtRoute( "GetMessageById", new { id = 42 }, null );
+
+        [HttpGet( "search" )]
+        public IActionResult Search( string query ) => Ok( new { Controller = GetType().Name, Query = query, Version = HttpContext.GetRequestedApiVersion().ToString() } );
     }
 }