Correct matching of action descriptors with same name, but different API versions. Fixes #117

Author: Chris Martinez

src/Microsoft.AspNet.WebApi.Versioning.ApiExplorer/CollectionExtensions.cs

@@ -0,0 +1,30 @@
+namespace Microsoft.Web.Http
+{
+    using System;
+    using System.Collections.Generic;
+    using System.Diagnostics.Contracts;
+
+    static class CollectionExtensions
+    {
+        internal static int IndexOf<TItem>( this IEnumerable<TItem> sequence, TItem item, IEqualityComparer<TItem> comparer )
+        {
+            Contract.Requires( sequence != null );
+            Contract.Requires( comparer != null );
+            Contract.Ensures( Contract.Result<int>() >= -1 );
+
+            var index = 0;
+
+            foreach ( var element in sequence )
+            {
+                if ( comparer.Equals( element, item ) )
+                {
+                    return index;
+                }
+
+                index++;
+            }
+
+            return -1;
+        }
+    }
+}

src/Microsoft.AspNet.WebApi.Versioning.ApiExplorer/Description/VersionedApiExplorer.cs

@@ -268,18 +268,32 @@ protected virtual ApiDescriptionGroupCollection InitializeApiDescriptions()
 
                     // Remove ApiDescription that will lead to ambiguous action matching.
                     // E.g. a controller with Post() and PostComment(). When the route template is {controller}, it produces POST /controller and POST /controller.
-                    descriptionsFromRoute = RemoveInvalidApiDescriptions( descriptionsFromRoute );
+                    descriptionsFromRoute = RemoveInvalidApiDescriptions( descriptionsFromRoute, apiVersion );
 
                     foreach ( var description in descriptionsFromRoute )
                     {
                         // Do not add the description if the previous route has a matching description with the same HTTP method and relative path.
                         // E.g. having two routes with the templates "api/Values/{id}" and "api/{controller}/{id}" can potentially produce the same
                         // relative path "api/Values/{id}" but only the first one matters.
-                        if ( !apiDescriptionGroup.ApiDescriptions.Contains( description, Comparer ) )
+
+                        var index = apiDescriptionGroup.ApiDescriptions.IndexOf( description, Comparer );
+
+                        if ( index < 0 )
                         {
                             description.GroupName = apiDescriptionGroup.Name;
                             apiDescriptionGroup.ApiDescriptions.Add( description );
                         }
+                        else
+                        {
+                            var model = description.ActionDescriptor.GetApiVersionModel();
+                            var overrideImplicitlyMappedApiDescription = model.DeclaredApiVersions.Contains( apiVersion );
+
+                            if ( overrideImplicitlyMappedApiDescription )
+                            {
+                                description.GroupName = apiDescriptionGroup.Name;
+                                apiDescriptionGroup.ApiDescriptions[index] = description;
+                            }
+                        }
                     }
 
                     if ( apiDescriptionGroup.ApiDescriptions.Count == 0 )
@@ -969,41 +983,34 @@ ApiParameterDescription CreateParameterDescriptionFromBinding( HttpParameterBind
             return parameterDescription;
         }
 
-        static Collection<VersionedApiDescription> RemoveInvalidApiDescriptions( Collection<VersionedApiDescription> apiDescriptions )
+        static Collection<VersionedApiDescription> RemoveInvalidApiDescriptions( Collection<VersionedApiDescription> apiDescriptions, ApiVersion apiVersion )
         {
             Contract.Requires( apiDescriptions != null );
+            Contract.Requires( apiVersion != null );
             Contract.Ensures( Contract.Result<Collection<VersionedApiDescription>>() != null );
 
-            var duplicateApiDescriptionIds = new HashSet<string>( StringComparer.OrdinalIgnoreCase );
-            var visitedApiDescriptionIds = new HashSet<string>( StringComparer.OrdinalIgnoreCase );
+            var filteredDescriptions = new Dictionary<string, VersionedApiDescription>( StringComparer.OrdinalIgnoreCase );
 
             foreach ( var description in apiDescriptions )
             {
                 var apiDescriptionId = description.GetUniqueID();
 
-                if ( visitedApiDescriptionIds.Contains( apiDescriptionId ) )
+                if ( filteredDescriptions.ContainsKey( apiDescriptionId ) )
                 {
-                    duplicateApiDescriptionIds.Add( apiDescriptionId );
+                    var model = description.ActionDescriptor.GetApiVersionModel();
+
+                    if ( model.DeclaredApiVersions.Contains( apiVersion ) )
+                    {
+                        filteredDescriptions[apiDescriptionId] = description;
+                    }
                 }
                 else
                 {
-                    visitedApiDescriptionIds.Add( apiDescriptionId );
-                }
-            }
-
-            var filteredApiDescriptions = new Collection<VersionedApiDescription>();
-
-            foreach ( var apiDescription in apiDescriptions )
-            {
-                var apiDescriptionId = apiDescription.GetUniqueID();
-
-                if ( !duplicateApiDescriptionIds.Contains( apiDescriptionId ) )
-                {
-                    filteredApiDescriptions.Add( apiDescription );
+                    filteredDescriptions.Add( apiDescriptionId, description );
                 }
             }
 
-            return filteredApiDescriptions;
+            return new Collection<VersionedApiDescription>( filteredDescriptions.Values.ToList() );
         }
 
         static bool MatchRegexConstraint( IHttpRoute route, string parameterName, string parameterValue )

test/Microsoft.AspNet.WebApi.Versioning.ApiExplorer.Tests/Description/TestConfigurations.cs

@@ -38,6 +38,7 @@ static HttpConfiguration NewConventionRouteConfiguration()
                                        .HasApiVersion( 2, 0 )
                                        .HasDeprecatedApiVersion( 3, 0, "beta" )
                                        .HasApiVersion( 3, 0 )
+                                       .Action( c => c.GetV3() ).MapToApiVersion( 3, 0 )
                                        .Action( c => c.Post( default( ClassWithId ) ) ).MapToApiVersion( 3, 0 );
                     options.Conventions.Controller<Values3Controller>()
                                        .HasApiVersion( 4, 0 )

test/Microsoft.AspNet.WebApi.Versioning.ApiExplorer.Tests/Description/VersionedApiExplorerTest.cs

@@ -356,21 +356,24 @@ public void api_description_group_should_explore_v3_actions( HttpConfiguration c
                         ID = $"GET{relativePaths[0]}",
                         HttpMethod = Get,
                         RelativePath = relativePaths[0],
-                        Version = apiVersion
+                        Version = apiVersion,
+                        ActionDescriptor = new { ActionName = "GetV3" }
                     },
                     new
                     {
                         ID = $"GET{relativePaths[1]}",
                         HttpMethod = Get,
                         RelativePath = relativePaths[1],
-                        Version = apiVersion
+                        Version = apiVersion,
+                        ActionDescriptor = new { ActionName = "Get" }
                     },
                     new
                     {
                         ID = $"POST{relativePaths[2]}",
                         HttpMethod = Post,
                         RelativePath = relativePaths[2],
-                        Version = apiVersion
+                        Version = apiVersion,
+                        ActionDescriptor = new { ActionName = "Post" }
                     }
                 },
                 options => options.ExcludingMissingMembers() );

test/Microsoft.AspNet.WebApi.Versioning.ApiExplorer.Tests/Simulators/AttributeValues2Controller.cs

@@ -1,6 +1,8 @@
 namespace Microsoft.Web.Http.Description.Simulators
 {
     using Microsoft.Web.Http.Description.Models;
+    using System;
+    using System.Web.Http.Description;
     using System.Web.Http;
 
     [ApiVersion( "2.0" )]
@@ -10,7 +12,12 @@
     public class AttributeValues2Controller : ApiController
     {
         [Route]
-        public IHttpActionResult Get() => Ok();
+        public string Get() => "Test";
+
+        [Route]
+        [MapToApiVersion( "3.0" )]
+        [ResponseType( typeof( string ) )]
+        public IHttpActionResult GetV3() => Ok( "Test" );
 
         [Route( "{id:int}" )]
         public IHttpActionResult Get( int id ) => Ok();

test/Microsoft.AspNet.WebApi.Versioning.ApiExplorer.Tests/Simulators/Values2Controller.cs

@@ -2,11 +2,15 @@
 {
     using Microsoft.Web.Http.Description.Models;
     using System.Web.Http;
+    using System.Web.Http.Description;
 
     [ControllerName( "Values" )]
     public class Values2Controller : ApiController
     {
-        public IHttpActionResult Get() => Ok();
+        public string Get() => "Test";
+
+        [ResponseType( typeof( string ) )]
+        public IHttpActionResult GetV3() => Ok( "Test" );
 
         public IHttpActionResult Get( int id ) => Ok();