Fix Mapping of Controller Actions to API Version Conventions

ApiVersioning.sln

@@ -87,6 +87,7 @@ Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNet.OData.Vers
 EndProject
 Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Conventions", "Conventions", "{B24995FB-AF48-4E5D-9327-377A599BDE2A}"
 	ProjectSection(SolutionItems) = preProject
+		src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderCollectionT.cs = src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderCollectionT.cs
 		src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderT.cs = src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderT.cs
 		src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderTExtensions.cs = src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderTExtensions.cs
 		src\Common\Versioning\Conventions\ActionConventionBuilderTExtensions.cs = src\Common\Versioning\Conventions\ActionConventionBuilderTExtensions.cs

ApiVersioningWithSamples.sln

@@ -116,6 +116,19 @@ Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNet.WebApi.Acc
 EndProject
 Project("{8BB2217D-0F2D-49D1-97BC-3654ED321F3B}") = "Microsoft.AspNetCore.Mvc.Acceptance.Tests", "test\Microsoft.AspNetCore.Mvc.Acceptance.Tests\Microsoft.AspNetCore.Mvc.Acceptance.Tests.xproj", "{4EED304C-D1A6-4866-8D7F-450D084FD25D}"
 EndProject
+Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "Conventions", "Conventions", "{2ABB1DE5-8E77-440D-9517-4A5E6877D1C5}"
+	ProjectSection(SolutionItems) = preProject
+		src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderCollectionT.cs = src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderCollectionT.cs
+		src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderT.cs = src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderT.cs
+		src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderTExtensions.cs = src\Common\Versioning\Conventions\ActionApiVersionConventionBuilderTExtensions.cs
+		src\Common\Versioning\Conventions\ActionConventionBuilderTExtensions.cs = src\Common\Versioning\Conventions\ActionConventionBuilderTExtensions.cs
+		src\Common\Versioning\Conventions\ControllerApiVersionConventionBuilderT.cs = src\Common\Versioning\Conventions\ControllerApiVersionConventionBuilderT.cs
+		src\Common\Versioning\Conventions\ControllerApiVersionConventionBuilderTExtensions.cs = src\Common\Versioning\Conventions\ControllerApiVersionConventionBuilderTExtensions.cs
+		src\Common\Versioning\Conventions\ExpressionExtensions.cs = src\Common\Versioning\Conventions\ExpressionExtensions.cs
+		src\Common\Versioning\Conventions\IActionConventionBuilderT.cs = src\Common\Versioning\Conventions\IActionConventionBuilderT.cs
+		src\Common\Versioning\Conventions\IApiVersionConventionT.cs = src\Common\Versioning\Conventions\IApiVersionConventionT.cs
+	EndProjectSection
+EndProject
 Global
 	GlobalSection(SolutionConfigurationPlatforms) = preSolution
 		Debug|Any CPU = Debug|Any CPU
@@ -214,5 +227,6 @@ Global
 		{1EFC221F-35CF-4B55-BD59-240D5B808E14} = {900DD210-8500-4D89-A05D-C9526935A719}
 		{5C31964D-EA8B-420B-9297-5ADFEFE54962} = {0987757E-4D09-4523-B9C9-65B1E8832AA1}
 		{4EED304C-D1A6-4866-8D7F-450D084FD25D} = {0987757E-4D09-4523-B9C9-65B1E8832AA1}
+		{2ABB1DE5-8E77-440D-9517-4A5E6877D1C5} = {DE4EE45F-F8EA-4B32-B16F-441F946ACEF4}
 	EndGlobalSection
 EndGlobal

src/Common/Versioning/Conventions/ActionApiVersionConventionBuilderCollectionT.cs

@@ -0,0 +1,108 @@
+#if WEBAPI
+namespace Microsoft.Web.Http.Versioning.Conventions
+#else
+namespace Microsoft.AspNetCore.Mvc.Versioning.Conventions
+#endif
+{
+    using System;
+    using System.Collections;
+    using System.Collections.Generic;
+    using System.Diagnostics.Contracts;
+    using System.Linq;
+    using System.Reflection;
+
+    /// <summary>
+    /// Represents a collection of controller action convention builders.
+    /// </summary>
+    public partial class ActionApiVersionConventionBuilderCollection<T> : IReadOnlyCollection<ActionApiVersionConventionBuilder<T>>
+    {
+        private readonly ControllerApiVersionConventionBuilder<T> controllerBuilder;
+        private readonly IList<ActionBuilderMapping<T>> actionBuilderMappings = new List<ActionBuilderMapping<T>>();
+
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ActionApiVersionConventionBuilderCollection{T}"/> class.
+        /// </summary>
+        /// <param name="controllerBuilder">The associated <see cref="ControllerApiVersionConventionBuilder{T}">controller convention builder</see>.</param>
+        public ActionApiVersionConventionBuilderCollection( ControllerApiVersionConventionBuilder<T> controllerBuilder )
+        {
+            Arg.NotNull( controllerBuilder, nameof( controllerBuilder ) );
+            this.controllerBuilder = controllerBuilder;
+        }
+
+        /// <summary>
+        /// Gets or adds a controller action convention builder for the specified method.
+        /// </summary>
+        /// <param name="actionMethod">The controller action method to get or add the convention builder for.</param>
+        /// <returns>A new or existing <see cref="ActionApiVersionConventionBuilder{T}">controller action convention builder</see>.</returns>
+        protected internal virtual ActionApiVersionConventionBuilder<T> GetOrAdd( MethodInfo actionMethod )
+        {
+            Arg.NotNull( actionMethod, nameof( actionMethod ) );
+
+            var mapping = actionBuilderMappings.FirstOrDefault( m => m.Method == actionMethod );
+
+            if ( mapping == null )
+            {
+                mapping = new ActionBuilderMapping<T>( actionMethod, new ActionApiVersionConventionBuilder<T>( controllerBuilder ) );
+                actionBuilderMappings.Add( mapping );
+            }
+
+            return mapping.Builder;
+        }
+
+        /// <summary>
+        /// Gets a count of the controller action convention builders in the collection.
+        /// </summary>
+        /// <value>The total number of controller action convention builders in the collection.</value>
+        public virtual int Count => actionBuilderMappings.Count;
+
+        /// <summary>
+        /// Attempts to retrieve the controller action convention builder for the specified method.
+        /// </summary>
+        /// <param name="actionMethod">The controller action method to get the convention builder for.</param>
+        /// <param name="actionBuilder">The <see cref="ActionApiVersionConventionBuilder{T}">controller action convention builder</see> or <c>null</c>.</param>
+        /// <returns>True if the <paramref name="actionBuilder">action builder</paramref> is successfully retrieved; otherwise, false.</returns>
+        public virtual bool TryGetValue( MethodInfo actionMethod, out ActionApiVersionConventionBuilder<T> actionBuilder )
+        {
+            actionBuilder = null;
+
+            if ( actionMethod == null )
+            {
+                return false;
+            }
+
+            var mapping = actionBuilderMappings.FirstOrDefault( m => m.Method == actionMethod );
+
+            return ( actionBuilder = mapping?.Builder ) != null;
+        }
+
+        /// <summary>
+        /// Returns an iterator that enumerates the controller action convention builders in the collection.
+        /// </summary>
+        /// <returns>An <see cref="IEnumerator{T}"/> object.</returns>
+        public virtual IEnumerator<ActionApiVersionConventionBuilder<T>> GetEnumerator()
+        {
+            foreach ( var mapping in actionBuilderMappings )
+            {
+                yield return mapping.Builder;
+            }
+        }
+
+        IEnumerator IEnumerable.GetEnumerator() => GetEnumerator();
+
+        private sealed partial class ActionBuilderMapping<TModel>
+        {
+            internal ActionBuilderMapping( MethodInfo method, ActionApiVersionConventionBuilder<TModel> builder )
+            {
+                Contract.Requires( method != null );
+                Contract.Requires( builder != null );
+
+                Method = method;
+                Builder = builder;
+            }
+
+            internal MethodInfo Method { get; }
+
+            internal ActionApiVersionConventionBuilder<TModel> Builder { get; }
+        }
+    }
+}

src/Common/Versioning/Conventions/ControllerApiVersionConventionBuilderT.cs

@@ -21,6 +21,14 @@ public partial class ControllerApiVersionConventionBuilder<T>
         private readonly HashSet<ApiVersion> advertisedVersions = new HashSet<ApiVersion>();
         private readonly HashSet<ApiVersion> deprecatedAdvertisedVersions = new HashSet<ApiVersion>();
 
+        /// <summary>
+        /// Initializes a new instance of the <see cref="ControllerApiVersionConventionBuilder{T}"/> class.
+        /// </summary>
+        public ControllerApiVersionConventionBuilder()
+        {
+            ActionBuilders = new ActionApiVersionConventionBuilderCollection<T>( this );
+        }
+
         /// <summary>
         /// Gets or sets a value indicating whether the current controller is API version-neutral.
         /// </summary>
@@ -54,10 +62,9 @@ public partial class ControllerApiVersionConventionBuilder<T>
         /// <summary>
         /// Gets a collection of controller action convention builders.
         /// </summary>
-        /// <value>A <see cref="IDictionary{TKey, TValue}">collection</see> of
+        /// <value>A <see cref="ActionApiVersionConventionBuilderCollection{T}">collection</see> of
         /// <see cref="ActionApiVersionConventionBuilder{T}">controller action convention builders</see>.</value>
-        protected IDictionary<int, ActionApiVersionConventionBuilder<T>> ActionBuilders { get; } =
-            new Dictionary<int, ActionApiVersionConventionBuilder<T>>();
+        protected virtual ActionApiVersionConventionBuilderCollection<T> ActionBuilders { get; }
 
         /// <summary>
         /// Indicates that the controller is API version-neutral.
@@ -136,16 +143,7 @@ public virtual ActionApiVersionConventionBuilder<T> Action( MethodInfo actionMet
         {
             Arg.NotNull( actionMethod, nameof( actionMethod ) );
             Contract.Ensures( Contract.Result<ActionApiVersionConventionBuilder<T>>() != null );
-
-            var key = actionMethod.GetHashCode();
-            var actionBuilder = default( ActionApiVersionConventionBuilder<T> );
-
-            if ( !ActionBuilders.TryGetValue( key, out actionBuilder ) )
-            {
-                ActionBuilders[key] = actionBuilder = new ActionApiVersionConventionBuilder<T>( this );
-            }
-
-            return actionBuilder;
+            return ActionBuilders.GetOrAdd( actionMethod );
         }
     }
 }

src/Microsoft.AspNet.OData.Versioning/project.json

@@ -8,7 +8,7 @@
 
     "dependencies": {
         "Microsoft.AspNet.OData": "[5.9.1,6.0.0)",
-        "Microsoft.AspNet.WebApi.Versioning": "2.0.1-*"
+        "Microsoft.AspNet.WebApi.Versioning": "2.0.2-*"
     },
 
     "frameworks": {

src/Microsoft.AspNet.WebApi.Versioning/Versioning/Conventions/ActionApiVersionConventionBuilderCollectionT.cs

@@ -0,0 +1,16 @@
+namespace Microsoft.Web.Http.Versioning.Conventions
+{
+    using System;
+    using System.Web.Http.Controllers;
+
+    /// <content>
+    /// Provides additional implementation specific to Microsoft ASP.NET Web API.
+    /// </content>
+    /// <typeparam name="T">The <see cref="Type">type</see> of <see cref="IHttpController">controller</see>.</typeparam>
+    public partial class ActionApiVersionConventionBuilderCollection<T> where T : IHttpController
+    {
+        private sealed partial class ActionBuilderMapping<TModel> where TModel : IHttpController
+        {
+        }
+    }
+}

src/Microsoft.AspNet.WebApi.Versioning/Versioning/Conventions/ControllerApiVersionConventionBuilderT.cs

@@ -84,7 +84,7 @@ private void ApplyActionConventions( HttpControllerDescriptor controllerDescript
 
             foreach ( var actionDescriptor in actionDescriptors )
             {
-                var key = actionDescriptor.MethodInfo.GetHashCode();
+                var key = actionDescriptor.MethodInfo;
                 var actionBuilder = default( ActionApiVersionConventionBuilder<T> );
 
                 if ( ActionBuilders.TryGetValue( key, out actionBuilder ) )

src/Microsoft.AspNet.WebApi.Versioning/project.json

@@ -1,5 +1,5 @@
 {
-    "version": "2.0.1-*",
+    "version": "2.0.2-*",
     "authors": [ "Microsoft" ],
     "title": "Microsoft ASP.NET Web API Versioning",
     "copyright": "Copyright \u00A9 2016. Microsoft Corporation. All rights reserved.",
@@ -26,7 +26,7 @@
         "summary": "Provides API versioning for RESTful services created using ASP.NET Web API",
         "tags": [ "Microsoft", "AspNet", "AspNetWebAPI", "Versioning" ],
         "owners": [ "Microsoft" ],
-        "releaseNotes": "\u2022 Fixed InvalidCastException in attribute-routing (Issue #44)",
+        "releaseNotes": "\u2022 Fixed mapping of API-versioned actions by convention (Issue #55)",
         "iconUrl": "http://go.microsoft.com/fwlink/?LinkID=288890",
         "licenseUrl": "https://raw.githubusercontent.com/Microsoft/aspnet-api-versioning/master/LICENSE",
         "requireLicenseAcceptance": true,

src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/Conventions/ActionApiVersionConventionBuilderCollectionT.cs

@@ -0,0 +1,14 @@
+namespace Microsoft.AspNetCore.Mvc.Versioning.Conventions
+{
+    using ApplicationModels;
+    using System;
+
+    /// <content>
+    /// Provides additional implementation specific to Microsoft ASP.NET Core.
+    /// </content>
+    /// <typeparam name="T">The <see cref="Type">type</see> of <see cref="ICommonModel">model</see>.</typeparam>
+    [CLSCompliant( false )]
+    public partial class ActionApiVersionConventionBuilderCollection<T> 
+    {
+    }
+}

src/Microsoft.AspNetCore.Mvc.Versioning/Versioning/Conventions/ControllerApiVersionConventionBuilderT.cs

@@ -89,7 +89,7 @@ private void ApplyActionConventions( ControllerModel controller, ControllerVersi
 
             foreach ( var action in controller.Actions )
             {
-                var key = action.ActionMethod.GetHashCode();
+                var key = action.ActionMethod;
                 var actionBuilder = default( ActionApiVersionConventionBuilder<T> );
 
                 action.SetProperty( controller );

src/Microsoft.AspNetCore.Mvc.Versioning/project.json

@@ -1,5 +1,5 @@
 {
-    "version": "1.0.1-*",
+    "version": "1.0.2-*",
     "authors": [ "Microsoft" ],
     "title": "Microsoft ASP.NET Core API Versioning",
     "copyright": "Copyright \u00A9 2015. Microsoft Corporation. All rights reserved.",
@@ -30,7 +30,7 @@
         "summary": "Provides API versioning for RESTful services created using ASP.NET Core",
         "tags": [ "Microsoft", "AspNet", "AspNetCore", "Versioning" ],
         "owners": [ "Microsoft" ],
-        "releaseNotes": "\u2022 Fixed API version reporting using conventions (Issue #47)\n\u2022 Removed unused ApiVersionActionSelector.ApiVersionReader",
+        "releaseNotes": "\u2022 Fixed ArgumentNullException in ApiVersionActionSelector (Issue #52)\n\u2022 Fixed mapping of API-versioned actions by convention (Issue #55)",
         "iconUrl": "http://go.microsoft.com/fwlink/?LinkID=288890",
         "licenseUrl": "https://raw.githubusercontent.com/Microsoft/aspnet-api-versioning/master/LICENSE",
         "requireLicenseAcceptance": true,

test/Microsoft.AspNet.WebApi.Versioning.Tests/Versioning/Conventions/ControllerApiVersionConventionBuilderTTest.cs

@@ -16,7 +16,7 @@ private sealed class TestControllerApiVersionConventionBuilder : ControllerApiVe
         {
             internal bool ProtectedVersionNeutral => VersionNeutral;
 
-            internal IDictionary<int, ActionApiVersionConventionBuilder<IHttpController>> ProtectedActionBuilders => ActionBuilders;
+            internal ActionApiVersionConventionBuilderCollection<IHttpController> ProtectedActionBuilders => ActionBuilders;
         }
 
         private sealed class UndecoratedController : ApiController
@@ -70,7 +70,7 @@ public void action_should_add_new_action_convention_builder()
             var actionBuilder = controllerBuilder.Action( method );
 
             // assert
-            controllerBuilder.ProtectedActionBuilders.Values.Single().Should().BeSameAs( actionBuilder );
+            controllerBuilder.ProtectedActionBuilders.Single().Should().BeSameAs( actionBuilder );
         }
 
         [Fact]
@@ -86,7 +86,7 @@ public void action_should_return_existing_action_convention_builder()
 
             // assert
             actionBuilder.Should().BeSameAs( originalActionBuilder );
-            controllerBuilder.ProtectedActionBuilders.Values.Single().Should().BeSameAs( actionBuilder );
+            controllerBuilder.ProtectedActionBuilders.Single().Should().BeSameAs( actionBuilder );
         }
 
         [Fact]

test/Microsoft.AspNetCore.Mvc.Acceptance.Tests/Conventions/given/a_query_string_versioned_Controller_split_into_two_types_using_conventions.cs

@@ -12,32 +12,22 @@
 
     public class _a_query_string_versioned_Controller_split_into_two_types_using_conventions : ConventionsAcceptanceTest
     {
-        [Fact]
-        public async Task _get_should_return_200()
+        [Theory]
+        [InlineData( nameof( ValuesController ), "1.0" )]
+        [InlineData( nameof( Values2Controller ), "2.0" )]
+        [InlineData( nameof( Values2Controller ), "3.0" )]
+        public async Task _get_should_return_200( string controller, string apiVersion )
         {
-            // REMARKS: this test should be a theory, but when it is, it becomes flaky. any failure succeeds when run again.
-            // the exact cause is unknown, but seems to be related to some form of caching. running a loop in a single test
-            // case seems to resolve the problem.
-
             // arrange
-            var iterations = new[]
-            {
-                new { Controller = nameof( ValuesController),  ApiVersion = "1.0" },
-                new { Controller = nameof( Values2Controller), ApiVersion = "2.0" },
-                new { Controller = nameof( Values2Controller), ApiVersion = "3.0" }
-            };
             var example = new { controller = "", version = "" };
 
-            foreach ( var iteration in iterations )
-            {
-                // act
-                var response = await GetAsync( $"api/values?api-version={iteration.ApiVersion}" ).EnsureSuccessStatusCode();
-                var content = await response.Content.ReadAsExampleAsync( example );
+            // act
+            var response = await GetAsync( $"api/values?api-version={apiVersion}" ).EnsureSuccessStatusCode();
+            var content = await response.Content.ReadAsExampleAsync( example );
 
-                // assert
-                response.Headers.GetValues( "api-supported-versions" ).Single().Should().Be( "1.0, 2.0, 3.0" );
-                content.ShouldBeEquivalentTo( new { controller = iteration.Controller, version = iteration.ApiVersion } );
-            }
+            // assert
+            response.Headers.GetValues( "api-supported-versions" ).Single().Should().Be( "1.0, 2.0, 3.0" );
+            content.ShouldBeEquivalentTo( new { controller = controller, version = apiVersion } );
         }
 
         [Fact]