Pre-Aggregate API Version Models

.editorconfig

@@ -13,11 +13,11 @@ indent_size = 4
 
 # xml project files
 [*.{csproj,vbproj,proj,projitems,shproj}]
-indent_size = 2
+indent_size = 1
 
 # xml config files
 [*.{props,targets,ruleset,config,nuspec,resx,vsixmanifest,vsct}]
-indent_size = 2
+indent_size = 1
 
 # json files
 [*.json]

build/common.props

@@ -7,6 +7,10 @@
   <Copyright>© $(Company). All rights reserved.</Copyright>
   <NeutralLanguage>en</NeutralLanguage>
   <DefaultLanguage>en-US</DefaultLanguage>
+  </PropertyGroup>
+
+ <PropertyGroup Label="C#">
+  <LangVersion>latest</LangVersion>
  </PropertyGroup>
 
 </Project>

src/Microsoft.AspNetCore.Mvc.Versioning/Abstractions/ActionDescriptorExtensions.cs

@@ -2,8 +2,6 @@
 {
     using ApplicationModels;
     using System;
-    using System.Collections.Generic;
-    using System.Diagnostics.Contracts;
     using System.Linq;
     using Versioning;
 
@@ -13,30 +11,6 @@
     [CLSCompliant( false )]
     public static class ActionDescriptorExtensions
     {
-        const string VersionPolicyIsAppliedKey = "MS_" + nameof( VersionPolicyIsApplied );
-
-        static void VersionPolicyIsApplied( this ActionDescriptor action, bool value ) => action.Properties[VersionPolicyIsAppliedKey] = value;
-
-        internal static bool VersionPolicyIsApplied( this ActionDescriptor action ) => action.Properties.GetOrDefault( VersionPolicyIsAppliedKey, false );
-
-        internal static void AggregateAllVersions( this ActionDescriptor action, IEnumerable<ActionDescriptor> matchingActions )
-        {
-            Contract.Requires( action != null );
-            Contract.Requires( matchingActions != null );
-
-            if ( action.VersionPolicyIsApplied() )
-            {
-                return;
-            }
-
-            action.VersionPolicyIsApplied( true );
-
-            var model = action.GetProperty<ApiVersionModel>();
-            Contract.Assume( model != null );
-
-            action.SetProperty( model.Aggregate( matchingActions.Select( a => a.GetProperty<ApiVersionModel>() ).Where( m => m != null ) ) );
-        }
-
         /// <summary>
         /// Returns a value indicating whether the provided action implicitly maps to the specified version.
         /// </summary>

src/Microsoft.AspNetCore.Mvc.Versioning/Microsoft.Extensions.DependencyInjection/IServiceCollectionExtensions.cs

@@ -7,6 +7,7 @@
     using Extensions;
     using Microsoft.AspNetCore.Builder;
     using Microsoft.AspNetCore.Hosting;
+    using Microsoft.AspNetCore.Mvc.Abstractions;
     using Options;
     using System;
     using System.Diagnostics.Contracts;
@@ -54,6 +55,7 @@ public static IServiceCollection AddApiVersioning( this IServiceCollection servi
             if ( options.ReportApiVersions )
             {
                 services.TryAddSingleton<IReportApiVersions, DefaultApiVersionReporter>();
+                services.AddTransient<IActionDescriptorProvider, ApiVersionCollator>();
             }
             else
             {

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

@@ -164,9 +164,7 @@ 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( candidates );
             context.RouteData = match.RouteData;
             context.Handler = handler.Invoke;
         }

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

@@ -166,6 +166,7 @@ public virtual ActionDescriptor SelectBestCandidate( RouteContext context, IRead
                 }
                 else
                 {
+                    AppendPossibleMatches( new[] { selectedAction }, context, selectionResult );
                     return selectedAction;
                 }
             }
@@ -261,7 +262,7 @@ static ActionDescriptor SelectActionWithApiVersionPolicyApplied( IReadOnlyList<A
 
             var match = matches[0];
 
-            if ( match.VersionPolicyIsApplied() && !result.HasMatchesInPreviousIterations )
+            if ( !result.HasMatchesInPreviousIterations )
             {
                 return match;
             }

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

@@ -0,0 +1,134 @@
+namespace Microsoft.AspNetCore.Mvc.Versioning
+{
+    using Microsoft.AspNetCore.Mvc.Abstractions;
+    using Microsoft.AspNetCore.Mvc.Controllers;
+    using System;
+    using System.Collections.Generic;
+    using System.Diagnostics.Contracts;
+    using System.Linq;
+
+    /// <summary>
+    /// Represents an object that collates <see cref="ApiVersion">API versions</see> per <see cref="ActionDescriptor">action</see>.
+    /// </summary>
+    [CLSCompliant( false )]
+    public class ApiVersionCollator : IActionDescriptorProvider
+    {
+        /// <inheritdoc />
+        public int Order { get; protected set; }
+
+        /// <inheritdoc />
+        public virtual void OnProvidersExecuted( ActionDescriptorProviderContext context )
+        {
+            foreach ( var actions in GroupActionsByController( context.Results ) )
+            {
+                var collatedModel = CollateModel( actions );
+
+                foreach ( var action in actions )
+                {
+                    var model = action.GetProperty<ApiVersionModel>();
+
+                    if ( model != null )
+                    {
+                        action.SetProperty( model.Aggregate( collatedModel ) );
+                    }
+                }
+            }
+        }
+
+        /// <inheritdoc />
+        public virtual void OnProvidersExecuting( ActionDescriptorProviderContext context )
+        {
+        }
+
+        /// <summary>
+        /// Resolves and returns the logical controller name for the specified action.
+        /// </summary>
+        /// <param name="action">The <see cref="ActionDescriptor">action</see> to get the controller name from.</param>
+        /// <returns>The logical name of the associated controller.</returns>
+        /// <remarks>
+        /// <para>
+        /// The logical controller name is used to collate actions together and aggregate API versions. The
+        /// default implementation uses the "controller" route parameter and falls back to the
+        /// <see cref="ControllerActionDescriptor.ControllerName"/> property when available.
+        /// </para>
+        /// <para>
+        /// The default implementation will also trim trailing numbers in the controller name by convention. For example,
+        /// the type "Values2Controller" will have the controller name "Values2", which will be trimmed to just "Values".
+        /// This behavior can be changed by using the <see cref="ControllerNameAttribute"/> or overriding the default
+        /// implementation.
+        /// </para>
+        /// </remarks>
+        protected virtual string GetControllerName( ActionDescriptor action )
+        {
+            Arg.NotNull( action, nameof( action ) );
+
+            if ( !action.RouteValues.TryGetValue( "controller", out var key ) )
+            {
+                if ( action is ControllerActionDescriptor controllerAction )
+                {
+                    key = controllerAction.ControllerName;
+                }
+            }
+
+            return TrimTrailingNumbers( key );
+        }
+
+        IEnumerable<IEnumerable<ActionDescriptor>> GroupActionsByController( IEnumerable<ActionDescriptor> actions )
+        {
+            Contract.Requires( actions != null );
+            Contract.Ensures( Contract.Result<IEnumerable<IEnumerable<ActionDescriptor>>>() != null );
+
+            var groups = new Dictionary<string, List<ActionDescriptor>>( StringComparer.OrdinalIgnoreCase );
+
+            foreach ( var action in actions )
+            {
+                var key = GetControllerName( action );
+
+                if ( string.IsNullOrEmpty( key ) )
+                {
+                    continue;
+                }
+
+                if ( !groups.TryGetValue( key, out var values ) )
+                {
+                    groups.Add( key, values = new List<ActionDescriptor>() );
+                }
+
+                values.Add( action );
+            }
+
+            foreach ( var value in groups.Values )
+            {
+                yield return value;
+            }
+        }
+
+        static string TrimTrailingNumbers( string name )
+        {
+            if ( string.IsNullOrEmpty( name ) )
+            {
+                return name;
+            }
+
+            var last = name.Length - 1;
+
+            for ( var i = last; i >= 0; i-- )
+            {
+                if ( !char.IsNumber( name[i] ) )
+                {
+                    if ( i < last )
+                    {
+                        return name.Substring( 0, i + 1 );
+                    }
+
+                    return name;
+                }
+            }
+
+            return name;
+        }
+
+        static ApiVersionModel CollateModel( IEnumerable<ActionDescriptor> actions ) =>
+            actions.Select( a => a.GetProperty<ApiVersionModel>() ).Where( m => m != null ).Aggregate();
+    }
+}

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

@@ -452,7 +452,7 @@ public async Task select_best_candidate_should_return_correct_controller_for_ver
             var deprecated = new[] { new ApiVersion( 4, 0 ) };
             var implemented = supported.Union( deprecated ).OrderBy( v => v ).ToArray();
 
-            using ( var server = new WebServer() )
+            using ( var server = new WebServer( o => o.ReportApiVersions = true ) )
             {
                 await server.Client.GetAsync( $"api/{versionSegment}/attributed" );
 

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

@@ -0,0 +1,140 @@
+namespace Microsoft.AspNetCore.Mvc.Versioning
+{
+    using FluentAssertions;
+    using Microsoft.AspNetCore.Mvc.Abstractions;
+    using Microsoft.AspNetCore.Mvc.Controllers;
+    using System.Collections.Generic;
+    using System.Linq;
+    using Xunit;
+
+    public class ApiVersionCollatorTest
+    {
+        [Theory]
+        [MemberData( nameof( ActionDescriptorProviderContexts ) )]
+        public void on_providers_executed_should_aggregate_api_version_models_by_controller( ActionDescriptorProviderContext context )
+        {
+            // arrange
+            var collator = new ApiVersionCollator();
+            var expected = new[] { new ApiVersion( 1, 0 ), new ApiVersion( 2, 0 ), new ApiVersion( 3, 0 ) };
+
+            // act
+            collator.OnProvidersExecuted( context );
+
+            // assert
+            var actions = context.Results.Where( a => a.GetProperty<ApiVersionModel>() != null );
+
+            actions.All( a => a.GetProperty<ApiVersionModel>().SupportedApiVersions.SequenceEqual( expected ) ).Should().BeTrue();
+        }
+
+        public static IEnumerable<object[]> ActionDescriptorProviderContexts
+        {
+            get
+            {
+                yield return new object[] { ActionsWithRouteValues };
+                yield return new object[] { ActionsByControllerName };
+            }
+        }
+
+        private static ActionDescriptorProviderContext ActionsWithRouteValues =>
+            new ActionDescriptorProviderContext()
+            {
+                Results =
+                {
+                    new ActionDescriptor()
+                    {
+                        RouteValues = new Dictionary<string, string>()
+                        {
+                            ["controller"] = "Values",
+                            ["action"] = "Get",
+                        },
+                        Properties = new Dictionary<object, object>()
+                        {
+                            [typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 1, 0 ) ),
+                        },
+                    },
+                    new ActionDescriptor()
+                    {
+                        RouteValues = new Dictionary<string, string>()
+                        {
+                            ["page"] = "/Some/Page",
+                        },
+                    },
+                    new ActionDescriptor()
+                    {
+                        RouteValues = new Dictionary<string, string>()
+                        {
+                            ["controller"] = "Values",
+                            ["action"] = "Get",
+                        },
+                        Properties = new Dictionary<object, object>()
+                        {
+                            [typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 2, 0 ) ),
+                        },
+                    },
+                    new ActionDescriptor()
+                    {
+                        RouteValues = new Dictionary<string, string>()
+                        {
+                            ["controller"] = "Values",
+                            ["action"] = "Get",
+                        },
+                        Properties = new Dictionary<object, object>()
+                        {
+                            [typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 3, 0 ) ),
+                        },
+                    },
+                },
+            };
+
+        private static ActionDescriptorProviderContext ActionsByControllerName =>
+            new ActionDescriptorProviderContext()
+            {
+                Results =
+                {
+                    new ControllerActionDescriptor()
+                    {
+                        ControllerName = "Values",
+                        RouteValues = new Dictionary<string, string>()
+                        {
+                            ["action"] = "Get",
+                        },
+                        Properties = new Dictionary<object, object>()
+                        {
+                            [typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 1, 0 ) ),
+                        },
+                    },
+                    new ActionDescriptor()
+                    {
+                        RouteValues = new Dictionary<string, string>()
+                        {
+                            ["page"] = "/Some/Page",
+                        },
+                    },
+                    new ControllerActionDescriptor()
+                    {
+                        ControllerName = "Values2",
+                        RouteValues = new Dictionary<string, string>()
+                        {
+                            ["action"] = "Get",
+                        },
+                        Properties = new Dictionary<object, object>()
+                        {
+                            [typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 2, 0 ) ),
+                        },
+                    },
+                    new ControllerActionDescriptor()
+                    {
+                        ControllerName = "Values3",
+                        RouteValues = new Dictionary<string, string>()
+                        {
+                            ["action"] = "Get",
+                        },
+                        Properties = new Dictionary<object, object>()
+                        {
+                            [typeof(ApiVersionModel)] = new ApiVersionModel( new ApiVersion( 3, 0 ) ),
+                        },
+                    },
+                },
+            };
+    }
+}